All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@linaro.org>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	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>,
	Alan Stern <stern@rowland.harvard.edu>,
	Mark Brown <broonie@kernel.org>,
	Russell King <linux@arm.linux.org.uk>,
	Linus Walleij <linus.walleij@linaro.org>,
	Wolfram Sang <wsa@the-dreams.de>,
	Alessandro Rubini <rubini@unipv.it>
Subject: Re: [PATCH 0/8] PM / Sleep / Runtime: Fixup some driver's system suspend
Date: Fri, 28 Feb 2014 09:21:34 -0800	[thread overview]
Message-ID: <7hk3cftaox.fsf@paris.lan> (raw)
In-Reply-To: <CAPDyKFp_ugRytLGR4fGXvdZEMRohg6uWPykcFJ7Co_wipJujDA@mail.gmail.com> (Ulf Hansson's message of "Thu, 27 Feb 2014 09:18:34 +0100")

Ulf Hansson <ulf.hansson@linaro.org> writes:

> On 27 February 2014 02:22, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> On Wednesday, February 26, 2014 08:30:07 AM Kevin Hilman wrote:
>>> Ulf Hansson <ulf.hansson@linaro.org> writes:
>>>
>>> > Patch 1 -> 2:
>>> > These patches provides two new runtime PM helper functions which intend to be
>>> > used from system suspend/resume callbacks, to make sure devices are put into low
>>> > power state during system suspend and brought back to full power at system
>>> > resume.
>>> >
>>> > The prerequisite is to have all levels of a device's runtime PM callbacks to be
>>> > defined through the SET_PM_RUNTIME_PM_OPS macro, which means these are available
>>> > for CONFIG_PM.
>>> >
>>> > By using the new runtime PM helper functions especially the two scenarios below
>>> > will be addressed.
>>> >
>>> > 1) The PM core prevents .runtime_suspend callbacks from being invoked during
>>> > system suspend. That means even for a runtime PM centric subsystem and driver,
>>> > the device needs to be put into low power state from a system suspend callback.
>>> > Otherwise it may very well be left in full power state (runtime resumed) while
>>> > the system is suspended. By using the new helper functions, we make sure to walk
>>> > the hierarchy of a device's power domain, subsystem and driver.
>>>
>>> I thought it was the case that runtime PM was only disabled during the
>>> 'late' phase now.  Isn't that enough to allow runtime callbacks in the
>>> normal suspend/resume hooks now?   /me looks.
>>>
>>> oh, wait.  Ee still have the _get_noresume() in device_prepare().  hmm
>>>
>>> Either way, I'm not not a big fan of new functions.  Personally, I think
>>> subsystems/busses/pm_domains should be able to opt out of the PM core
>>> behavior that blocks runtime PM transitions during system suspend.
>>> Something like the (untested) hack below.  That way, we could avoid the
>>> need for new helper functions.
>>
>> And if one of the subsystems in question is the platform bus type, then
>> adding the flag doesn't really make sense, because that means that on
>> many system it will be set for the majority of devices. :-)
>>
>> That said I'm tired of this stuff already.  If you really really want to
>> remove the bumping up and dropping of the usage counter during system
>> suspend/resume by the core, please feel free to submit a patch for that
>> and I'll apply it.  However, if it causes any regressions to happen
>> anywhere, it will be dropped and we'll never talk about this again.
>>
>> Deal?
>
> No deal. I prefer the runtime PM helpers, I think that is the right
> approach. I also believe this is what Alan Stern also would prefer,
> right!? So let's convince Kevin instead. :-)

I don't need much convincing.  

My preference is for fewer functions/helpers because runtime PM is
already complicated enough for drivers.  However, I'm not going to
object to the helpers because they will allow us to simplify many
drivers/subsystems that are trying to handle the various combinations of
suspend/resume and runtime PM, so it's a step in the right direction.

Kevin

WARNING: multiple messages have this Message-ID (diff)
From: khilman@linaro.org (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 0/8] PM / Sleep / Runtime: Fixup some driver's system suspend
Date: Fri, 28 Feb 2014 09:21:34 -0800	[thread overview]
Message-ID: <7hk3cftaox.fsf@paris.lan> (raw)
In-Reply-To: <CAPDyKFp_ugRytLGR4fGXvdZEMRohg6uWPykcFJ7Co_wipJujDA@mail.gmail.com> (Ulf Hansson's message of "Thu, 27 Feb 2014 09:18:34 +0100")

Ulf Hansson <ulf.hansson@linaro.org> writes:

> On 27 February 2014 02:22, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> On Wednesday, February 26, 2014 08:30:07 AM Kevin Hilman wrote:
>>> Ulf Hansson <ulf.hansson@linaro.org> writes:
>>>
>>> > Patch 1 -> 2:
>>> > These patches provides two new runtime PM helper functions which intend to be
>>> > used from system suspend/resume callbacks, to make sure devices are put into low
>>> > power state during system suspend and brought back to full power at system
>>> > resume.
>>> >
>>> > The prerequisite is to have all levels of a device's runtime PM callbacks to be
>>> > defined through the SET_PM_RUNTIME_PM_OPS macro, which means these are available
>>> > for CONFIG_PM.
>>> >
>>> > By using the new runtime PM helper functions especially the two scenarios below
>>> > will be addressed.
>>> >
>>> > 1) The PM core prevents .runtime_suspend callbacks from being invoked during
>>> > system suspend. That means even for a runtime PM centric subsystem and driver,
>>> > the device needs to be put into low power state from a system suspend callback.
>>> > Otherwise it may very well be left in full power state (runtime resumed) while
>>> > the system is suspended. By using the new helper functions, we make sure to walk
>>> > the hierarchy of a device's power domain, subsystem and driver.
>>>
>>> I thought it was the case that runtime PM was only disabled during the
>>> 'late' phase now.  Isn't that enough to allow runtime callbacks in the
>>> normal suspend/resume hooks now?   /me looks.
>>>
>>> oh, wait.  Ee still have the _get_noresume() in device_prepare().  hmm
>>>
>>> Either way, I'm not not a big fan of new functions.  Personally, I think
>>> subsystems/busses/pm_domains should be able to opt out of the PM core
>>> behavior that blocks runtime PM transitions during system suspend.
>>> Something like the (untested) hack below.  That way, we could avoid the
>>> need for new helper functions.
>>
>> And if one of the subsystems in question is the platform bus type, then
>> adding the flag doesn't really make sense, because that means that on
>> many system it will be set for the majority of devices. :-)
>>
>> That said I'm tired of this stuff already.  If you really really want to
>> remove the bumping up and dropping of the usage counter during system
>> suspend/resume by the core, please feel free to submit a patch for that
>> and I'll apply it.  However, if it causes any regressions to happen
>> anywhere, it will be dropped and we'll never talk about this again.
>>
>> Deal?
>
> No deal. I prefer the runtime PM helpers, I think that is the right
> approach. I also believe this is what Alan Stern also would prefer,
> right!? So let's convince Kevin instead. :-)

I don't need much convincing.  

My preference is for fewer functions/helpers because runtime PM is
already complicated enough for drivers.  However, I'm not going to
object to the helpers because they will allow us to simplify many
drivers/subsystems that are trying to handle the various combinations of
suspend/resume and runtime PM, so it's a step in the right direction.

Kevin

  reply	other threads:[~2014-02-28 17:21 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-20 15:31 [PATCH 0/8] PM / Sleep / Runtime: Fixup some driver's system suspend Ulf Hansson
2014-02-20 15:31 ` Ulf Hansson
2014-02-20 15:31 ` [PATCH 1/8] PM / Runtime: Fetch runtime PM callbacks using a macro Ulf Hansson
2014-02-20 15:31   ` Ulf Hansson
2014-02-20 16:28   ` Josh Cartwright
2014-02-20 16:28     ` Josh Cartwright
2014-02-26 15:50   ` Kevin Hilman
2014-02-26 15:50     ` Kevin Hilman
2014-02-26 22:02     ` Ulf Hansson
2014-02-26 22:02       ` Ulf Hansson
2014-02-20 15:31 ` [PATCH 2/8] PM / Sleep / Runtime: Add pm_runtime_suspend|resume_force functions Ulf Hansson
2014-02-20 15:31   ` Ulf Hansson
2014-02-20 15:31 ` [PATCH 3/8] spi: pl022: Let runtime PM callbacks be available for CONFIG_PM Ulf Hansson
2014-02-20 15:31   ` Ulf Hansson
2014-02-20 17:25   ` Josh Cartwright
2014-02-20 17:25     ` Josh Cartwright
2014-02-20 15:31 ` [PATCH 4/8] spi: pl022: Don't ignore power domain and amba bus at system suspend Ulf Hansson
2014-02-20 15:31   ` Ulf Hansson
2014-02-20 15:31 ` [PATCH 5/8] i2c: nomadik: Fixup " Ulf Hansson
2014-02-20 15:31   ` Ulf Hansson
2014-02-20 15:31 ` [PATCH 6/8] mmc: mmci: Mask IRQs for all variants during runtime suspend Ulf Hansson
2014-02-20 15:31   ` Ulf Hansson
2014-02-20 15:31 ` [PATCH 7/8] mmc: mmci: Let runtime PM callbacks be available for CONFIG_PM Ulf Hansson
2014-02-20 15:31   ` Ulf Hansson
2014-02-20 15:31 ` [PATCH 8/8] mmc: mmci: Put the device into low power state at system suspend Ulf Hansson
2014-02-20 15:31   ` Ulf Hansson
2014-02-26 16:30 ` [PATCH 0/8] PM / Sleep / Runtime: Fixup some driver's " Kevin Hilman
2014-02-26 16:30   ` Kevin Hilman
2014-02-26 22:30   ` Ulf Hansson
2014-02-26 22:30     ` Ulf Hansson
2014-02-27  1:22   ` Rafael J. Wysocki
2014-02-27  1:22     ` Rafael J. Wysocki
2014-02-27  8:18     ` Ulf Hansson
2014-02-27  8:18       ` Ulf Hansson
2014-02-28 17:21       ` Kevin Hilman [this message]
2014-02-28 17:21         ` Kevin Hilman

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=7hk3cftaox.fsf@paris.lan \
    --to=khilman@linaro.org \
    --cc=broonie@kernel.org \
    --cc=len.brown@intel.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=pavel@ucw.cz \
    --cc=rjw@rjwysocki.net \
    --cc=rubini@unipv.it \
    --cc=stern@rowland.harvard.edu \
    --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.