All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Walmsley <paul@pwsan.com>
To: Nishanth Menon <nm@ti.com>
Cc: Tony Lindgren <tony@atomide.com>,
	rnayak@ti.com, khilman@linaro.org, balbi@ti.com,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH V3] ARM: OMAP2+: omap_device: maintain sane runtime pm status around suspend/resume
Date: Fri, 15 Nov 2013 08:07:34 +0000 (UTC)	[thread overview]
Message-ID: <alpine.DEB.2.02.1311150806220.15980@utopia.booyaka.com> (raw)
In-Reply-To: <1384448716-3186-1-git-send-email-nm@ti.com>

On Thu, 14 Nov 2013, Nishanth Menon wrote:

> OMAP device hooks around suspend|resume_noirq ensures that hwmod
> devices are forced to idle using omap_device_idle/enable as part of
> the last stage of suspend activity.
> 
> For a device such as i2c who uses autosuspend, it is possible to enter
> the suspend path with dev->power.runtime_status = RPM_ACTIVE.
> 
> As part of the suspend flow, the generic runtime logic would increment
> it's dev->power.disable_depth to 1. This should prevent further
> pm_runtime_get_sync from succeeding once the runtime_status has been
> set to RPM_SUSPENDED.
> 
> Now, as part of the suspend_noirq handler in omap_device, we force the
> following: if the device status is !suspended, we force the device
> to idle using omap_device_idle (clocks are cut etc..). This ensures
> that from a hardware perspective, the device is "suspended". However,
> runtime_status is left to be active.
> 
> *if* an operation is attempted after this point to
> pm_runtime_get_sync, runtime framework depends on runtime_status to
> indicate accurately the device status, and since it sees it to be
> ACTIVE, it assumes the module is functional and returns a non-error
> value. As a result the user will see pm_runtime_get succeed, however a
> register access will crash due to the lack of clocks.
> 
> To prevent this from happening, we should ensure that runtime_status
> exactly indicates the device status. As a result of this change
> any further calls to pm_runtime_get* would return -EACCES (since
> disable_depth is 1). On resume, we restore the clocks and runtime
> status exactly as we suspended with. These operations are not expected
> to fail as we update the states after the core runtime framework has
> suspended itself and restore before the core runtime framework has
> resumed.
> 
> Reported-by: J Keerthy <j-keerthy@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Acked-by: Rajendra Nayak <rnayak@ti.com>
> Acked-by: Kevin Hilman <khilman@linaro.org>

Looks reasonable to me.  Looks like this should be considered for -stable 
- Nishanth, what do you think?

Tony or Kevin, do you want to take this one, or want me to?


- Paul

WARNING: multiple messages have this Message-ID (diff)
From: paul@pwsan.com (Paul Walmsley)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V3] ARM: OMAP2+: omap_device: maintain sane runtime pm status around suspend/resume
Date: Fri, 15 Nov 2013 08:07:34 +0000 (UTC)	[thread overview]
Message-ID: <alpine.DEB.2.02.1311150806220.15980@utopia.booyaka.com> (raw)
In-Reply-To: <1384448716-3186-1-git-send-email-nm@ti.com>

On Thu, 14 Nov 2013, Nishanth Menon wrote:

> OMAP device hooks around suspend|resume_noirq ensures that hwmod
> devices are forced to idle using omap_device_idle/enable as part of
> the last stage of suspend activity.
> 
> For a device such as i2c who uses autosuspend, it is possible to enter
> the suspend path with dev->power.runtime_status = RPM_ACTIVE.
> 
> As part of the suspend flow, the generic runtime logic would increment
> it's dev->power.disable_depth to 1. This should prevent further
> pm_runtime_get_sync from succeeding once the runtime_status has been
> set to RPM_SUSPENDED.
> 
> Now, as part of the suspend_noirq handler in omap_device, we force the
> following: if the device status is !suspended, we force the device
> to idle using omap_device_idle (clocks are cut etc..). This ensures
> that from a hardware perspective, the device is "suspended". However,
> runtime_status is left to be active.
> 
> *if* an operation is attempted after this point to
> pm_runtime_get_sync, runtime framework depends on runtime_status to
> indicate accurately the device status, and since it sees it to be
> ACTIVE, it assumes the module is functional and returns a non-error
> value. As a result the user will see pm_runtime_get succeed, however a
> register access will crash due to the lack of clocks.
> 
> To prevent this from happening, we should ensure that runtime_status
> exactly indicates the device status. As a result of this change
> any further calls to pm_runtime_get* would return -EACCES (since
> disable_depth is 1). On resume, we restore the clocks and runtime
> status exactly as we suspended with. These operations are not expected
> to fail as we update the states after the core runtime framework has
> suspended itself and restore before the core runtime framework has
> resumed.
> 
> Reported-by: J Keerthy <j-keerthy@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Acked-by: Rajendra Nayak <rnayak@ti.com>
> Acked-by: Kevin Hilman <khilman@linaro.org>

Looks reasonable to me.  Looks like this should be considered for -stable 
- Nishanth, what do you think?

Tony or Kevin, do you want to take this one, or want me to?


- Paul

  parent reply	other threads:[~2013-11-15  8:07 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-12 23:08 [PATCH V2] ARM: OMAP2+: omap_device: maintain sane runtime pm status around suspend/resume Nishanth Menon
2013-11-12 23:08 ` Nishanth Menon
2013-11-12 23:08 ` Nishanth Menon
2013-11-13 12:51 ` Felipe Balbi
2013-11-13 12:51   ` Felipe Balbi
2013-11-13 12:51   ` Felipe Balbi
2013-11-13 14:56   ` Nishanth Menon
2013-11-13 14:56     ` Nishanth Menon
2013-11-13 14:56     ` Nishanth Menon
2013-11-13 15:20     ` Felipe Balbi
2013-11-13 15:20       ` Felipe Balbi
2013-11-13 15:20       ` Felipe Balbi
2013-11-13 15:28       ` Nishanth Menon
2013-11-13 15:28         ` Nishanth Menon
2013-11-13 15:28         ` Nishanth Menon
2013-11-13 15:45     ` Kevin Hilman
2013-11-13 15:45       ` Kevin Hilman
2013-11-13 15:45       ` Kevin Hilman
2013-11-13 16:19       ` Nishanth Menon
2013-11-13 16:19         ` Nishanth Menon
2013-11-13 16:19         ` Nishanth Menon
2013-11-14 17:05 ` [PATCH V3] " Nishanth Menon
2013-11-14 17:05   ` Nishanth Menon
2013-11-14 17:05   ` Nishanth Menon
2013-11-14 18:55   ` Felipe Balbi
2013-11-14 18:55     ` Felipe Balbi
2013-11-14 18:55     ` Felipe Balbi
2013-11-14 19:30     ` Nishanth Menon
2013-11-14 19:30       ` Nishanth Menon
2013-11-14 19:30       ` Nishanth Menon
2013-11-15  8:07   ` Paul Walmsley [this message]
2013-11-15  8:07     ` Paul Walmsley
2013-11-15 13:30     ` Nishanth Menon
2013-11-15 13:30       ` Nishanth Menon
2013-11-15 13:30       ` Nishanth Menon
2013-11-15 14:37       ` Tony Lindgren
2013-11-15 14:37         ` Tony Lindgren
2013-11-15 20:04         ` Paul Walmsley
2013-11-15 20:04           ` Paul Walmsley
2013-11-15 22:03         ` Kevin Hilman
2013-11-15 22:03           ` Kevin Hilman
2013-11-19 11:34           ` Ulf Hansson
2013-11-19 11:34             ` Ulf Hansson
2013-11-19 11:34             ` Ulf Hansson

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=alpine.DEB.2.02.1311150806220.15980@utopia.booyaka.com \
    --to=paul@pwsan.com \
    --cc=balbi@ti.com \
    --cc=khilman@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=rnayak@ti.com \
    --cc=tony@atomide.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.