Hi all,
Here are some PM runtime regression fixes for omaps after recent
pm_runtime_reinit changes.
We basically have to use pm_runtime_put_sync_suspend() or
a combination of pm_runtime_dont_use_autosuspend() and
pm_runtime_put_sync() if the driver has enabled autoidle.
If no objections, I'd like to merge this whole series via
the ARM SoC tree.
Regards,
Tony
Tony Lindgren (7):
mmc: omap_hsmmc: Fix PM regression with deferred probe for
pm_runtime_reinit
i2c: omap: Fix PM regression with deferred probe for pm_runtime_reinit
spi: omap2-mcspi: Fix PM regression with deferred probe for
pm_runtime_reinit
serial: 8250_omap: Fix PM regression with deferred probe for
pm_runtime_reinit
serial: omap: Fix PM regression with deferred probe for
pm_runtime_reinit
ARM: OMAP2+: Improve omap_device error for driver writers
ARM: OMAP2+: Fix omap_device for module reload on PM runtime forbid
arch/arm/mach-omap2/omap_device.c | 14 +++++++++++++-
drivers/i2c/busses/i2c-omap.c | 4 +++-
drivers/mmc/host/omap_hsmmc.c | 6 ++++--
drivers/spi/spi-omap2-mcspi.c | 3 +++
drivers/tty/serial/8250/8250_omap.c | 4 +++-
drivers/tty/serial/omap-serial.c | 4 +++-
6 files changed, 29 insertions(+), 6 deletions(-)
--
2.7.0
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-mmc at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Ulf, I'd like to merge this along with other related fixes via the
ARM SoC tree if no objections, please review and ack if this look OK
to you.
---
drivers/mmc/host/omap_hsmmc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
@@ -1247,7 +1247,7 @@ static int omap_hsmmc_switch_opcond(struct omap_hsmmc_host *host, int vdd)intret;/* Disable the clocks */-pm_runtime_put_sync(host->dev);+pm_runtime_put_sync_suspend(host->dev);if(host->dbclk)clk_disable_unprepare(host->dbclk);
On 11 February 2016 at 00:02, Tony Lindgren [off-list ref] wrote:
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-mmc at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Ulf, I'd like to merge this along with other related fixes via the
ARM SoC tree if no objections, please review and ack if this look OK
to you.
I have some other omap_hsmmc patches queued for 4.6, so I prefer to
send this via my mmc tree.
I guess that's okay as well!?
@@ -1247,7 +1247,7 @@ static int omap_hsmmc_switch_opcond(struct omap_hsmmc_host *host, int vdd)intret;/* Disable the clocks */-pm_runtime_put_sync(host->dev);+pm_runtime_put_sync_suspend(host->dev);
This has no effect.
The mmc core holds a runtime PM usage count for the device anyway, so
the clock won't be disabled.
Please remove this change from the patch.
quoted hunk
if (host->dbclk)
clk_disable_unprepare(host->dbclk);
@@ -2232,6 +2232,7 @@ err_irq: dma_release_channel(host->tx_chan); if (host->rx_chan) dma_release_channel(host->rx_chan);+ pm_runtime_dont_use_autosuspend(host->dev); pm_runtime_put_sync(host->dev); pm_runtime_disable(host->dev); if (host->dbclk)
@@ -2285,7 +2287,7 @@ static int omap_hsmmc_suspend(struct device *dev) if (host->dbclk) clk_disable_unprepare(host->dbclk);- pm_runtime_put_sync(host->dev);+ pm_runtime_put_sync_suspend(host->dev);
This has no effect, as the PM core prevents runtime suspend during the
system PM phase.
It does so, by increasing the runtime PM usage count
(pm_runtime_get_noresume()).
Please remove this change from patch.
On 11 February 2016 at 16:02, Tony Lindgren [off-list ref] wrote:
quoted
* Ulf Hansson [off-list ref] [160211 02:19]:
quoted
On 11 February 2016 at 00:02, Tony Lindgren [off-list ref] wrote:
quoted
Ulf, I'd like to merge this along with other related fixes via the
ARM SoC tree if no objections, please review and ack if this look OK
to you.
I have some other omap_hsmmc patches queued for 4.6, so I prefer to
send this via my mmc tree.
I guess that's okay as well!?
Naturally yes, please go ahead thanks!
Okay!
BTW, did you notice my other comments to the patch?
I can fix them before applying, unless you want to send a v2?
Oh sorry, no I missed the rest of your comments again, I really
need to recalibrate my reading habits obviously if I keep
missing comments on regular basis..
You comments make sense to me. Below is the patch with only
minimal changes. Seems to work just fine and hit off mode for
suspend too.
Regards,
Tony
8< -------------------
From eb3c93421f01fb6198f76127262a92b527dd214c Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Tue, 9 Feb 2016 09:31:10 -0800
Subject: [PATCH] mmc: omap_hsmmc: Fix PM regression with deferred probe for
pm_runtime_reinit
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-mmc at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
On 11 February 2016 at 18:26, Tony Lindgren [off-list ref] wrote:
quoted hunk
* Ulf Hansson [off-list ref] [160211 07:14]:
quoted
On 11 February 2016 at 16:02, Tony Lindgren [off-list ref] wrote:
quoted
* Ulf Hansson [off-list ref] [160211 02:19]:
quoted
On 11 February 2016 at 00:02, Tony Lindgren [off-list ref] wrote:
quoted
Ulf, I'd like to merge this along with other related fixes via the
ARM SoC tree if no objections, please review and ack if this look OK
to you.
I have some other omap_hsmmc patches queued for 4.6, so I prefer to
send this via my mmc tree.
I guess that's okay as well!?
Naturally yes, please go ahead thanks!
Okay!
BTW, did you notice my other comments to the patch?
I can fix them before applying, unless you want to send a v2?
Oh sorry, no I missed the rest of your comments again, I really
need to recalibrate my reading habits obviously if I keep
missing comments on regular basis..
You comments make sense to me. Below is the patch with only
minimal changes. Seems to work just fine and hit off mode for
suspend too.
Regards,
Tony
8< -------------------
From eb3c93421f01fb6198f76127262a92b527dd214c Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Tue, 9 Feb 2016 09:31:10 -0800
Subject: [PATCH] mmc: omap_hsmmc: Fix PM regression with deferred probe for
pm_runtime_reinit
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-mmc at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-i2c at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Cc: Wolfram Sang <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Wolfram, I'd like to merge this along with other related fixes via
the ARM SoC tree if no objections, please review and ack if this
look OK to you.
---
drivers/i2c/busses/i2c-omap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
From: Wolfram Sang <hidden> Date: 2016-02-11 17:34:28
Wolfram, I'd like to merge this along with other related fixes via
the ARM SoC tree if no objections, please review and ack if this
look OK to you.
Well, since most patches seem to be picked up individually now, I'd
prefer to do the same. This is v4.5 material, or?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160211/1252e49f/attachment.sig>
Wolfram, I'd like to merge this along with other related fixes via
the ARM SoC tree if no objections, please review and ack if this
look OK to you.
Well, since most patches seem to be picked up individually now, I'd
prefer to do the same. This is v4.5 material, or?
Sure please do, thanks. And yes for v4.5 please. Although
I've only seen this happen with the MMC driver so far, there's
still a slight chance of this happening with the other drivers
using omap_device.
Regards,
Tony
On 11 February 2016 at 00:02, Tony Lindgren [off-list ref] wrote:
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-i2c at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Cc: Wolfram Sang <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Wolfram, I'd like to merge this along with other related fixes via
the ARM SoC tree if no objections, please review and ack if this
look OK to you.
---
drivers/i2c/busses/i2c-omap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
From: Wolfram Sang <hidden> Date: 2016-02-12 18:43:36
On Wed, Feb 10, 2016 at 03:02:45PM -0800, Tony Lindgren wrote:
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-i2c at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Cc: Wolfram Sang <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-spi at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Mark, I'd like to merge this along with other related fixes via
the ARM SoC tree if no objections, please review and ack if this
look OK to you.
---
drivers/spi/spi-omap2-mcspi.c | 3 +++
1 file changed, 3 insertions(+)
On Wed, Feb 10, 2016 at 03:02:46PM -0800, Tony Lindgren wrote:
Mark, I'd like to merge this along with other related fixes via
the ARM SoC tree if no objections, please review and ack if this
look OK to you.
Why, I'm not seeing any dependencies here? I'd also expect to be seeing
an awful lot more changes like this, this shouldn't be an OMAP thing -
do we need to fix the core API and then roll out the transition in a
different way?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160211/735c9e60/attachment.sig>
On Wed, Feb 10, 2016 at 03:02:46PM -0800, Tony Lindgren wrote:
quoted
Mark, I'd like to merge this along with other related fixes via
the ARM SoC tree if no objections, please review and ack if this
look OK to you.
Why, I'm not seeing any dependencies here? I'd also expect to be seeing
an awful lot more changes like this, this shouldn't be an OMAP thing -
do we need to fix the core API and then roll out the transition in a
different way?
Please feel free to pick up this one if you have other fixes
lined up, nothing stopping that.
There will be likely more fixes like this to make drivers follow
the PM runtime API documentation. These were just the initial
ones that were obvious.
Regards,
Tony
On Thu, Feb 11, 2016 at 07:08:06AM -0800, Tony Lindgren wrote:
* Mark Brown [off-list ref] [160211 03:52]:
quoted
Why, I'm not seeing any dependencies here? I'd also expect to be seeing
an awful lot more changes like this, this shouldn't be an OMAP thing -
do we need to fix the core API and then roll out the transition in a
different way?
Please feel free to pick up this one if you have other fixes
lined up, nothing stopping that.
I'll take it myself unless there is a strong reason to merge it as part
of another series.
There will be likely more fixes like this to make drivers follow
the PM runtime API documentation. These were just the initial
ones that were obvious.
This does sound like there's been a change in the interface compared to
what users are actually doing - is this an actual problem or is it just
a divergence from docs?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160211/290ead7c/attachment-0001.sig>
On Thu, Feb 11, 2016 at 07:08:06AM -0800, Tony Lindgren wrote:
quoted
* Mark Brown [off-list ref] [160211 03:52]:
quoted
quoted
Why, I'm not seeing any dependencies here? I'd also expect to be seeing
an awful lot more changes like this, this shouldn't be an OMAP thing -
do we need to fix the core API and then roll out the transition in a
different way?
quoted
Please feel free to pick up this one if you have other fixes
lined up, nothing stopping that.
I'll take it myself unless there is a strong reason to merge it as part
of another series.
OK thanks!
quoted
There will be likely more fixes like this to make drivers follow
the PM runtime API documentation. These were just the initial
ones that were obvious.
This does sound like there's been a change in the interface compared to
what users are actually doing - is this an actual problem or is it just
a divergence from docs?
It's an actual problem at least on omaps as the omap_device code
is very picky about the hardware state.
Depending how the PM runtime is implemented, it may be a problem
for some other cases too.
For non-omap cases, my guess is that it's mostly a divergence from
the docs and non-critical.
Regards,
Tony
On Thu, Feb 11, 2016 at 09:36:20AM -0800, Tony Lindgren wrote:
* Mark Brown [off-list ref] [160211 07:54]:
quoted
This does sound like there's been a change in the interface compared to
what users are actually doing - is this an actual problem or is it just
a divergence from docs?
It's an actual problem at least on omaps as the omap_device code
is very picky about the hardware state.
Depending how the PM runtime is implemented, it may be a problem
for some other cases too.
Or people just aren't testing mainline that well (if this is broken in
v4.5 that suggests nobody noticed in -next) - do you know when this
broke? It really seems like we may need to spin round on how this is
deployed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160211/f1717419/attachment.sig>
On Thu, Feb 11, 2016 at 09:36:20AM -0800, Tony Lindgren wrote:
quoted
* Mark Brown [off-list ref] [160211 07:54]:
quoted
quoted
This does sound like there's been a change in the interface compared to
what users are actually doing - is this an actual problem or is it just
a divergence from docs?
quoted
It's an actual problem at least on omaps as the omap_device code
is very picky about the hardware state.
quoted
Depending how the PM runtime is implemented, it may be a problem
for some other cases too.
Or people just aren't testing mainline that well (if this is broken in
v4.5 that suggests nobody noticed in -next) - do you know when this
broke? It really seems like we may need to spin round on how this is
deployed.
I noticed it only with v4.5-rc1 and bisected it to commit 5de85b9d57ab
("PM / runtime: Re-init runtime PM states at probe error and driver
unbind") as it broke my PM tests for n900. For Linux next probably
was also broken for before that for some time.
Unfortunately based on the regressions I'm chasing every merge window
I'm suspecting that very few people are actually testing PM runtime
with mainline Linux kernel. Or they don't have the PM runtime fully
implemented in the mainline kernel for their devices. This is at
least for the SoC PM use case.
Ulf is working on a generic PM runtime test driver :) I'm hoping we
can then use that for basic regression testing on in an arch
independent way.
Regards,
Tony
On 11 February 2016 at 00:02, Tony Lindgren [off-list ref] wrote:
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-spi at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Mark, I'd like to merge this along with other related fixes via
the ARM SoC tree if no objections, please review and ack if this
look OK to you.
---
drivers/spi/spi-omap2-mcspi.c | 3 +++
1 file changed, 3 insertions(+)
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-serial at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Peter Hurley <redacted>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Greg & Peter, I'd like to merge this along with other related fixes
via the ARM SoC tree if no objections, please review and ack if this
look OK to you.
---
drivers/tty/serial/8250/8250_omap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
On Wed, Feb 10, 2016 at 03:02:47PM -0800, Tony Lindgren wrote:
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-serial at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Peter Hurley <redacted>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Greg & Peter, I'd like to merge this along with other related fixes
via the ARM SoC tree if no objections, please review and ack if this
look OK to you.
On 11 February 2016 at 00:02, Tony Lindgren [off-list ref] wrote:
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-serial at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Peter Hurley <redacted>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Greg & Peter, I'd like to merge this along with other related fixes
via the ARM SoC tree if no objections, please review and ack if this
look OK to you.
---
drivers/tty/serial/8250/8250_omap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-serial at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Greg & Peter, I'd like to merge this along with other related fixes
via the ARM SoC tree if no objections, please review and ack if
this look OK to you.
---
drivers/tty/serial/omap-serial.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
On 11 February 2016 at 00:02, Tony Lindgren [off-list ref] wrote:
Commit 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind") introduced pm_runtime_reinit() that is used
to reinitialize PM runtime after -EPROBE_DEFER. This allows shutting
down the device after a failed probe.
However, for drivers using pm_runtime_use_autosuspend() this can cause
a state where suspend callback is never called after -EPROBE_DEFER.
On the following device driver probe, hardware state is different from
the PM runtime state causing omap_device to produce the following
error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
The solution is to fix the drivers to follow the PM runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Fixes: 5de85b9d57ab ("PM / runtime: Re-init runtime PM states at probe
error and driver unbind")
Cc: linux-serial at vger.kernel.org
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Greg & Peter, I'd like to merge this along with other related fixes
via the ARM SoC tree if no objections, please review and ack if
this look OK to you.
---
drivers/tty/serial/omap-serial.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Drivers using pm_runtime_use_autosuspend() may not get disabled after
-EPROBE_DEFER. On the following device driver probe, hardware state
is different from the PM runtime state causing omap_device to produce
the following error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
Let's add a proper error message so driver writers can easily fix
their drivers for PM.
In general, the solution is to fix the drivers to follow the PM
runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Let's not return with 0 from _od_runtime_resume() as that will
eventually lead into new drivers with broken PM runtime that will
block deeper idle states on omaps.
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/mach-omap2/omap_device.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
On 11 February 2016 at 00:02, Tony Lindgren [off-list ref] wrote:
Drivers using pm_runtime_use_autosuspend() may not get disabled after
-EPROBE_DEFER. On the following device driver probe, hardware state
is different from the PM runtime state causing omap_device to produce
the following error:
omap_device_enable() called from invalid state 1
And with omap_device and omap hardware being picky for PM, this will
block any deeper idle states in hardware.
Let's add a proper error message so driver writers can easily fix
their drivers for PM.
In general, the solution is to fix the drivers to follow the PM
runtime documentation:
1. For sections of code that needs the device disabled, use
pm_runtime_put_sync_suspend() if pm_runtime_set_autosuspend() has
been set.
2. For driver exit code, use pm_runtime_dont_use_autosuspend() before
pm_runtime_put_sync() if pm_runtime_use_autosuspend() has been
set.
Let's not return with 0 from _od_runtime_resume() as that will
eventually lead into new drivers with broken PM runtime that will
block deeper idle states on omaps.
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Ulf Hansson <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
If a driver PM runtime is disabled via sysfs, and the module is
unloaded, PM runtime can't do anything to disable the device. Let's
let the interconnect disable the device on BUS_NOTIFY_UNBOUND_DRIVER.
Otherwise omap_device will produce and error on the following module
reload. This can be easily tested with something like:
# modprobe omap_hsmmc
# echo on > /sys/devices/platform/68000000.ocp/4809c000.mmc/power/control
# rmmod omap_hsmmc
# modprobe omap_hsmmc
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Tero Kristo <redacted>
Reported-by: Ulf Hansson <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/mach-omap2/omap_device.c | 10 ++++++++++
1 file changed, 10 insertions(+)
@@ -191,12 +191,22 @@ static int _omap_device_notifier_call(struct notifier_block *nb,{structplatform_device*pdev=to_platform_device(dev);structomap_device*od;+interr;switch(event){caseBUS_NOTIFY_DEL_DEVICE:if(pdev->archdata.od)omap_device_delete(pdev->archdata.od);break;+caseBUS_NOTIFY_UNBOUND_DRIVER:+od=to_omap_device(pdev);+if(od&&(od->_state==OMAP_DEVICE_STATE_ENABLED)){+dev_info(dev,"enabled after unload, idling\n");+err=omap_device_idle(pdev);+if(err)+dev_err(dev,"failed to idle\n");+}+break;caseBUS_NOTIFY_ADD_DEVICE:if(pdev->dev.of_node)omap_device_build_from_dt(pdev);
On 11 February 2016 at 00:02, Tony Lindgren [off-list ref] wrote:
If a driver PM runtime is disabled via sysfs, and the module is
unloaded, PM runtime can't do anything to disable the device. Let's
let the interconnect disable the device on BUS_NOTIFY_UNBOUND_DRIVER.
Otherwise omap_device will produce and error on the following module
reload. This can be easily tested with something like:
# modprobe omap_hsmmc
# echo on > /sys/devices/platform/68000000.ocp/4809c000.mmc/power/control
# rmmod omap_hsmmc
# modprobe omap_hsmmc
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Tero Kristo <redacted>
Reported-by: Ulf Hansson <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
@@ -191,12 +191,22 @@ static int _omap_device_notifier_call(struct notifier_block *nb,{structplatform_device*pdev=to_platform_device(dev);structomap_device*od;+interr;switch(event){caseBUS_NOTIFY_DEL_DEVICE:if(pdev->archdata.od)omap_device_delete(pdev->archdata.od);break;+caseBUS_NOTIFY_UNBOUND_DRIVER:+od=to_omap_device(pdev);+if(od&&(od->_state==OMAP_DEVICE_STATE_ENABLED)){+dev_info(dev,"enabled after unload, idling\n");+err=omap_device_idle(pdev);+if(err)+dev_err(dev,"failed to idle\n");+}+break;caseBUS_NOTIFY_ADD_DEVICE:if(pdev->dev.of_node)omap_device_build_from_dt(pdev);--