PM runtime regression fixes for omaps

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 <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---

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(-)

drivers/mmc/host/omap_hsmmc.c +0 -0

index b6639ea..32bc112 100644

drivers/mmc/host/omap_hsmmc.c +2 -0

        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)

@@ -2253,6 +2254,7 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
        dma_release_channel(host->tx_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);
        device_init_wakeup(&pdev->dev, false);

drivers/mmc/host/omap_hsmmc.c +0 -1

        return 0;
 }
 
--

2.7.0

Quoted 4 lines · Tony Lindgren

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!?

Quoted 6 lines · Ulf Hansson

Quoted 4 lines · Tony Lindgren

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.

Quoted 4 lines

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!

Quoted 12 lines · Tony Lindgren

Quoted 10 lines · Ulf Hansson

Quoted 4 lines · Tony Lindgren

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!?

Quoted 1 line

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?

From eb3c93421f01fb6198f76127262a92b527dd214c Mon Sep 17 00:00:00 2001

Quoted 13 lines · Ulf Hansson

Quoted 12 lines · Tony Lindgren

Quoted 10 lines · Ulf Hansson

Quoted 4 lines · Tony Lindgren

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!?

Quoted 7 lines

Quoted 1 line

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 <[email protected]>
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 <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

drivers/mmc/host/omap_hsmmc.c +1 -0

@@ -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)

Quoted 2 lines · Tony Lindgren

Subject: [PATCH] mmc: omap_hsmmc: Fix PM regression with deferred probe for
pm_runtime_reinit

Thanks, applied for fixes!

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.

Quoted 4 lines

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?

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 <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Cc Wolfram Sang <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---

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(-)

drivers/i2c/busses/i2c-omap.c +3 -2

index 08d26ba..13c4529 100644
@@ -1450,7 +1450,8 @@ omap_i2c_probe(struct platform_device *pdev)
 
 err_unuse_clocks:
        omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);
-       pm_runtime_put(omap->dev);
+       pm_runtime_dont_use_autosuspend(omap->dev);
+       pm_runtime_put_sync(omap->dev);
        pm_runtime_disable(&pdev->dev);
 err_free_mem:
 

@@ -1468,6 +1469,7 @@ static int omap_i2c_remove(struct platform_device *pdev)
                return ret;
 
        omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);
+       pm_runtime_dont_use_autosuspend(&pdev->dev);
        pm_runtime_put_sync(&pdev->dev);
        pm_runtime_disable(&pdev->dev);
        return 0;
--

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-i2c at vger.kernel.org

Cc Alan Stern <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Cc Wolfram Sang <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

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.

Quoted 3 lines · Tony Lindgren

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?

Quoted 4 lines · Mark Brown

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.

Quoted 6 lines · Tony Lindgren

Quoted 4 lines · Mark Brown

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 2 lines

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.

Quoted 3 lines

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?

Quoted 3 lines · Mark Brown

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.

Quoted 8 lines · Tony Lindgren

Quoted 3 lines · Mark Brown

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.

Quoted 2 lines

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.

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 <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Mark Brown <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---

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(+)

drivers/spi/spi-omap2-mcspi.c +3 -1

index 7273820..0caa3c8 100644
@@ -1490,6 +1490,8 @@ static int omap2_mcspi_probe(struct platform_device *pdev)
        return status;
 
 disable_pm:
+       pm_runtime_dont_use_autosuspend(&pdev->dev);
+       pm_runtime_put_sync(&pdev->dev);
        pm_runtime_disable(&pdev->dev);
 free_master:
        spi_master_put(master);

@@ -1501,6 +1503,7 @@ static int omap2_mcspi_remove(struct platform_device *pdev)
        struct spi_master *master = platform_get_drvdata(pdev);
        struct omap2_mcspi *mcspi = spi_master_get_devdata(master);
 
+       pm_runtime_dont_use_autosuspend(mcspi->dev);
        pm_runtime_put_sync(mcspi->dev);
        pm_runtime_disable(&pdev->dev);
 
--

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-serial at vger.kernel.org

Cc Alan Stern <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Peter Hurley <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---

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.

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 <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Peter Hurley <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---

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(-)

drivers/tty/serial/8250/8250_omap.c +3 -2

index a2c0734..81ff337 100644
@@ -1235,7 +1235,8 @@ static int omap8250_probe(struct platform_device *pdev)
        pm_runtime_put_autosuspend(&pdev->dev);
        return 0;
 err:
-       pm_runtime_put(&pdev->dev);
+       pm_runtime_dont_use_autosuspend(&pdev->dev);
+       pm_runtime_put_sync(&pdev->dev);
        pm_runtime_disable(&pdev->dev);
        return ret;
 }

@@ -1244,6 +1245,7 @@ static int omap8250_remove(struct platform_device *pdev)
 {
        struct omap8250_priv *priv = platform_get_drvdata(pdev);
 
+       pm_runtime_dont_use_autosuspend(&pdev->dev);
        pm_runtime_put_sync(&pdev->dev);
        pm_runtime_disable(&pdev->dev);
        serial8250_unregister_port(priv->line);
--

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-serial at vger.kernel.org

Cc Alan Stern <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---

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/tty/serial/omap-serial.c +3 -2

index b645f92..5c43f75 100644
@@ -1708,7 +1708,8 @@ static int serial_omap_probe(struct platform_device *pdev)
        return 0;
 
 err_add_port:
-       pm_runtime_put(&pdev->dev);
+       pm_runtime_dont_use_autosuspend(&pdev->dev);
+       pm_runtime_put_sync(&pdev->dev);
        pm_runtime_disable(&pdev->dev);
        pm_qos_remove_request(&up->pm_qos_request);
        device_init_wakeup(up->dev, false);

@@ -1721,6 +1722,7 @@ static int serial_omap_remove(struct platform_device *dev)
 {
        struct uart_omap_port *up = platform_get_drvdata(dev);
 
+       pm_runtime_dont_use_autosuspend(up->dev);
        pm_runtime_put_sync(up->dev);
        pm_runtime_disable(up->dev);
        uart_remove_one_port(&serial_omap_reg, &up->port);
--

2.7.0

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 <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 arch/arm/mach-omap2/omap_device.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

arch/arm/mach-omap2/omap_device.c +3 -1

index 0437537..ebd8369 100644
@@ -602,8 +602,10 @@ static int _od_runtime_resume(struct device *dev)
 	int ret;
 
 	ret = omap_device_enable(pdev);
-	if (ret)
+	if (ret) {
+		dev_err(dev, "use pm_runtime_put_sync_suspend() in driver?\n");
 		return ret;
+	}
 
 	return pm_generic_runtime_resume(dev);
 }

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 <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Tero Kristo <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 arch/arm/mach-omap2/omap_device.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

arch/arm/mach-omap2/omap_device.c +3 -2

index 0437537..ebd8369 100644
@@ -602,8 +602,10 @@ static int _od_runtime_resume(struct device *dev)
        int ret;
 
        ret = omap_device_enable(pdev);
-       if (ret)
+       if (ret) {
+               dev_err(dev, "use pm_runtime_put_sync_suspend() in driver?\n");
                return ret;
+       }
 
        return pm_generic_runtime_resume(dev);
 }
--

2.7.0

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 <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Tero Kristo <[email protected]>

Reported-by Ulf Hansson <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 arch/arm/mach-omap2/omap_device.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

arch/arm/mach-omap2/omap_device.c +10 -0

index ebd8369..f7ff3b9 100644
@@ -191,12 +191,22 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct omap_device *od;
+	int err;
 
 	switch (event) {
 	case BUS_NOTIFY_DEL_DEVICE:
 		if (pdev->archdata.od)
 			omap_device_delete(pdev->archdata.od);
 		break;
+	case BUS_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;
 	case BUS_NOTIFY_ADD_DEVICE:
 		if (pdev->dev.of_node)
 			omap_device_build_from_dt(pdev);

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 <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Nishanth Menon <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Tero Kristo <[email protected]>

Reported-by Ulf Hansson <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

---
 arch/arm/mach-omap2/omap_device.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

arch/arm/mach-omap2/omap_device.c +10 -1

index ebd8369..f7ff3b9 100644
@@ -191,12 +191,22 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
 {
        struct platform_device *pdev = to_platform_device(dev);
        struct omap_device *od;
+       int err;
 
        switch (event) {
        case BUS_NOTIFY_DEL_DEVICE:
                if (pdev->archdata.od)
                        omap_device_delete(pdev->archdata.od);
                break;
+       case BUS_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;
        case BUS_NOTIFY_ADD_DEVICE:
                if (pdev->dev.of_node)
                        omap_device_build_from_dt(pdev);
--

2.7.0