All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	Russell King <linux@arm.linux.org.uk>,
	Dan Williams <dan.j.williams@intel.com>,
	Vinod Koul <vinod.koul@intel.com>,
	Ulf Hansson <ulf.hansson@linaro.org>, <linux-pm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <dmaengine@vger.kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Michal Simek <michal.simek@xilinx.com>,
	Kevin Hilman <khilman@kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME
Date: Fri, 7 Nov 2014 09:50:58 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.1411070948180.1214-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <1415347601.8532.7.camel@AMDC1943>

On Fri, 7 Nov 2014, Krzysztof Kozlowski wrote:

> > Well, that is a good reason to introduce a wrapper around power.irq_safe in my
> > view.
> > 
> > And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
> > unset.
> > 
> > This way not only you wouldn't need to move the flag from under the #ifdef,
> > but also you would make the compiler skip the relevant pieces of code
> > entiretly for CONFIG_PM_RUNTIME unset.
> 
> Few days ago I would be happy with your opinion :), but know I think
> this is better solution than wrapper. Consider case:
> 1. PM_RUNTIME unset.
> 2. System suspends.
> 3. The pl330 in its suspend callback calls force_runtime_suspend which
> leads us to amba/bus.
> 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
> so it disables and unprepares the clock.
> 5. The pl330 in probe requested irq_safe so it assumes amba/bus will
> only disable the clock. So the pl330 unprepares the clock. Again.

To me, this sounds like a good reason to avoid using 
force_runtime_suspend().  In fact, it sounds like a good reason to 
avoid relying on the runtime PM mechanism to handle non-runtime-PM 
things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't 
enabled then the runtime PM stack simply should not be used.

Alan Stern


WARNING: multiple messages have this Message-ID (diff)
From: Alan Stern <stern@rowland.harvard.edu>
To: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	Russell King <linux@arm.linux.org.uk>,
	Dan Williams <dan.j.williams@intel.com>,
	Vinod Koul <vinod.koul@intel.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	dmaengine@vger.kernel.org, Lars-Peter Clausen <lars@metafoo.de>,
	Michal Simek <michal.simek@xilinx.com>,
	Kevin Hilman <khilman@kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME
Date: Fri, 7 Nov 2014 09:50:58 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.1411070948180.1214-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <1415347601.8532.7.camel@AMDC1943>

On Fri, 7 Nov 2014, Krzysztof Kozlowski wrote:

> > Well, that is a good reason to introduce a wrapper around power.irq_safe in my
> > view.
> > 
> > And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
> > unset.
> > 
> > This way not only you wouldn't need to move the flag from under the #ifdef,
> > but also you would make the compiler skip the relevant pieces of code
> > entiretly for CONFIG_PM_RUNTIME unset.
> 
> Few days ago I would be happy with your opinion :), but know I think
> this is better solution than wrapper. Consider case:
> 1. PM_RUNTIME unset.
> 2. System suspends.
> 3. The pl330 in its suspend callback calls force_runtime_suspend which
> leads us to amba/bus.
> 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
> so it disables and unprepares the clock.
> 5. The pl330 in probe requested irq_safe so it assumes amba/bus will
> only disable the clock. So the pl330 unprepares the clock. Again.

To me, this sounds like a good reason to avoid using 
force_runtime_suspend().  In fact, it sounds like a good reason to 
avoid relying on the runtime PM mechanism to handle non-runtime-PM 
things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't 
enabled then the runtime PM stack simply should not be used.

Alan Stern

  reply	other threads:[~2014-11-07 14:51 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-06  8:36 [PATCH v10 0/5] amba/dmaengine: pl330: add Power Management support Krzysztof Kozlowski
2014-11-06  8:36 ` [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME Krzysztof Kozlowski
2014-11-06 10:00   ` Ulf Hansson
2014-11-06 22:51   ` Rafael J. Wysocki
2014-11-07  8:06     ` Krzysztof Kozlowski
2014-11-07 14:50       ` Alan Stern [this message]
2014-11-07 14:50         ` Alan Stern
2014-11-07 23:45         ` Rafael J. Wysocki
2014-11-10 14:11         ` Ulf Hansson
2014-11-10 16:36           ` Alan Stern
2014-11-10 16:36             ` Alan Stern
2014-11-10 18:35             ` Ulf Hansson
2014-11-10 18:59               ` Alan Stern
2014-11-12  8:56                 ` Krzysztof Kozlowski
2014-11-12 10:39                 ` Ulf Hansson
2014-11-12 10:44                   ` Krzysztof Kozlowski
2014-11-12 16:34                   ` Alan Stern
2014-11-13 10:22                     ` Ulf Hansson
2014-11-13 15:55                       ` Alan Stern
2014-11-10 13:38       ` Ulf Hansson
2014-11-10 13:43   ` Srikanth K
2014-11-10 13:43     ` Srikanth K
2014-11-06  8:36 ` [PATCH v10 2/5] amba: Add helpers for (un)preparing AMBA clock Krzysztof Kozlowski
2014-11-06  8:36 ` [PATCH v10 3/5] amba: Don't unprepare the clocks if device driver wants IRQ safe runtime PM Krzysztof Kozlowski
2014-11-06 10:03   ` Ulf Hansson
2014-11-06 22:52   ` Rafael J. Wysocki
2014-11-07  8:01     ` Krzysztof Kozlowski
2014-11-07 23:52       ` Rafael J. Wysocki
2014-11-06  8:36 ` [PATCH v10 4/5] dmaengine: pl330: add Power Management support Krzysztof Kozlowski
2014-11-06 12:45   ` Vinod Koul
2014-11-06  8:36 ` [PATCH v10 5/5] amba: Remove unused amba_pclk_enable/disable macros Krzysztof Kozlowski

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=Pine.LNX.4.44L0.1411070948180.1214-100000@iolanthe.rowland.org \
    --to=stern@rowland.harvard.edu \
    --cc=b.zolnierkie@samsung.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=k.kozlowski@samsung.com \
    --cc=khilman@kernel.org \
    --cc=kyungmin.park@samsung.com \
    --cc=lars@metafoo.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=m.szyprowski@samsung.com \
    --cc=michal.simek@xilinx.com \
    --cc=pavel@ucw.cz \
    --cc=rjw@rjwysocki.net \
    --cc=ulf.hansson@linaro.org \
    --cc=vinod.koul@intel.com \
    /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.