Skip to contentSkip to conversation

Fixup use of runtime pm

PATCH Superseded Ulf Hansson 6 participants 16 messages Sep 28, 2012 – Oct 3, 2012

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover Fixup use of runtime pm

From: Ulf Hansson <[email protected]>

Some old runtime pm patches got merged whiched messed up things. These is now reverted. Additionaly one patch do a simplification of the use of runtime pm functions.

Ulf Hansson (3):
Revert "spi/pl022: fix spi-pl022 pm enable at probe" Revert "spi/pl022: enable runtime PM"
spi: spi-pl022: Minor simplification for runtime pm

 drivers/spi/spi-pl022.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
Signature · 1 line
1.7.10
#1 lore raw

PATCH 1/3 Revert "spi/pl022: fix spi-pl022 pm enable at probe"

From: Ulf Hansson <[email protected]>

This reverts commit 6887237cd7da904184dab2750504040c68f3a080.

---
 drivers/spi/spi-pl022.c |    1 +
 1 file changed, 1 insertion(+)
drivers/spi/spi-pl022.c +1 -0
index f8568b4..3f2f36c 100644
@@ -2188,6 +2188,7 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
 	printk(KERN_INFO "pl022: mapped registers from 0x%08x to %p\n",
 	       adev->res.start, pl022->virtbase);
 
+	pm_runtime_enable(dev);
 	pm_runtime_resume(dev);
 
 	pl022->clk = clk_get(&adev->dev, NULL);
Signature · 1 line
1.7.10
#2 lore raw

PATCH 1/3 Revert "spi/pl022: fix spi-pl022 pm enable at probe"

Hello Ulf,

Quoted 14 lines · Ulf HANSSON

From: Ulf Hansson<[email protected]>

This reverts commit 6887237cd7da904184dab2750504040c68f3a080.

---
  drivers/spi/spi-pl022.c |    1 +
  1 file changed, 1 insertion(+)
drivers/spi/spi-pl022.c +0 -0
index f8568b4..3f2f36c 100644

drivers/spi/spi-pl022.c pl022_probe(struct amba_device *adev, const struct amba_id *id) · line 2191

Quoted 4 lines
drivers/spi/spi-pl022.c +1 -0
@@ -2188,6 +2188,7 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
  	printk(KERN_INFO "pl022: mapped registers from 0x%08x to %p\n",
  	       adev->res.start, pl022->virtbase);
 
+	pm_runtime_enable(dev);

Before calling spi-pl022 probe, amba_probe enables runtime pm. Is it requires to re-enable it in spi-pl022 probe??

Regards
Vipul Samar

#3 lore raw

PATCH 1/3 Revert "spi/pl022: fix spi-pl022 pm enable at probe"

In reply to Ulf Hansson
Quoted 5 lines · Ulf Hansson

From: Ulf Hansson <[email protected]>

This reverts commit 6887237cd7da904184dab2750504040c68f3a080.

Why?

It was removed in this commit, is it wrong?

Author: Michel JAOUEN <[email protected]>
Date:   Fri Aug 17 17:28:41 2012 +0200

spi/pl022: fix spi-pl022 pm enable at probe

amba drivers does not need to enable pm runtime at probe. amba_probe already enables pm runtime.

This rids this warning in the ux500 boot log:
ssp-pl022 ssp0: Unbalanced pm_runtime_enable!

Yours,
Linus Walleij

#4 lore raw

PATCH 1/3 Revert "spi/pl022: fix spi-pl022 pm enable at probe"

Quoted 11 lines · Linus Walleij
Quoted 5 lines · Ulf Hansson

From: Ulf Hansson <[email protected]>

This reverts commit 6887237cd7da904184dab2750504040c68f3a080.

Why?

It was removed in this commit, is it wrong?

Linus, please read the previous thread on pl022.

The long and short of it is that your commit 2fb30d1147c599f5657e8c62c862f9a0f58d9d99 (spi/pl022: enable runtime PM) is totally broken, and Michel came along trying to fix the resulting breakage by partially removing your commit in 6887237cd7da904184dab2750504040c68f3a080 (spi/pl022: fix spi-pl022 pm enable at probe).

The reason why your commit is wrong is:

                pm_runtime_get_noresume(dev);
                pm_runtime_set_active(dev);
                pm_runtime_enable(dev);

                ret = pcdrv->probe(pcdev, id);

So, adding this in spi-pl022 probe:

+ pm_runtime_enable(dev);
+ pm_runtime_resume(dev);

makes the sequence:

	pm_runtime_get_noresume(dev);
	pm_runtime_set_active(dev);
	pm_runtime_enable(dev);
	pm_runtime_enable(dev);
	pm_runtime_resume(dev);

which is absolute rubbish. Your commit should never have gone into any tree.

The real answer is to revert both commits to get the driver back to a sane state, before then progressing with the driver in a sane manner. This is what Ulf is doing.

#5 lore raw

PATCH 1/3 Revert "spi/pl022: fix spi-pl022 pm enable at probe"

Quoted 3 lines · Russell King - ARM Linux

The real answer is to revert both commits to get the driver back to a sane state, before then progressing with the driver in a sane manner. This is what Ulf is doing.

Aha, mea culpa.

For all of them, I'll read up on this exploding backlog and then Ulf will hammer me at the office until I understand this stuff properly...

Yours,
Linus Walleij

#6 lore raw

PATCH 1/3 Revert "spi/pl022: fix spi-pl022 pm enable at probe"

In reply to Ulf Hansson

Hi Mark and Grant,

Heard from Linus Walleij that you Mark has been helping out merging spi patches, did not know that when sending out this series. Anyway, do you guys see any issues merging this?

[PATCH 1/3] Revert "spi/pl022: fix spi-pl022 pm enable at probe" [PATCH 2/3] Revert "spi/pl022: enable runtime PM" [PATCH 3/3] spi: spi-pl022: Minor simplification for runtime pm

Kind regards
Ulf Hansson

Quoted 24 lines · Ulf Hansson

From: Ulf Hansson <[email protected]>

This reverts commit 6887237cd7da904184dab2750504040c68f3a080.

---
 drivers/spi/spi-pl022.c |    1 +
 1 file changed, 1 insertion(+)
drivers/spi/spi-pl022.c +1 -1
index f8568b4..3f2f36c 100644
@@ -2188,6 +2188,7 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
        printk(KERN_INFO "pl022: mapped registers from 0x%08x to %p\n",
               adev->res.start, pl022->virtbase);
 
+       pm_runtime_enable(dev);
        pm_runtime_resume(dev);
 
        pl022->clk = clk_get(&adev->dev, NULL);
--

1.7.10

#7 lore raw

PATCH 1/3 Revert "spi/pl022: fix spi-pl022 pm enable at probe"

Quoted 7 lines · Ulf Hansson

Heard from Linus Walleij that you Mark has been helping out merging spi patches, did not know that when sending out this series. Anyway, do you guys see any issues merging this?

[PATCH 1/3] Revert "spi/pl022: fix spi-pl022 pm enable at probe" [PATCH 2/3] Revert "spi/pl022: enable runtime PM" [PATCH 3/3] spi: spi-pl022: Minor simplification for runtime pm

If someone could send the patches I'll take a look... some explanation as to why we're reverting things would be helpful.

#8 lore raw

PATCH 1/3 Revert "spi/pl022: fix spi-pl022 pm enable at probe"

Hi Mark,

I will do a resend and include some explanation for the reverts in the "cover-letter".

Kind regards
Ulf Hansson

Quoted 12 lines · Mark Brown
Quoted 7 lines · Ulf Hansson

Heard from Linus Walleij that you Mark has been helping out merging spi patches, did not know that when sending out this series. Anyway, do you guys see any issues merging this?

[PATCH 1/3] Revert "spi/pl022: fix spi-pl022 pm enable at probe" [PATCH 2/3] Revert "spi/pl022: enable runtime PM" [PATCH 3/3] spi: spi-pl022: Minor simplification for runtime pm

If someone could send the patches I'll take a look... some explanation as to why we're reverting things would be helpful.

#9 lore raw

PATCH 1/3 Revert "spi/pl022: fix spi-pl022 pm enable at probe"

Quoted 2 lines · Ulf Hansson

I will do a resend and include some explanation for the reverts in the "cover-letter".

It'd be good if the explanation could be in the commit logs so that people reading history can understand things.

#10 lore raw

PATCH 2/3 Revert "spi/pl022: enable runtime PM"

In reply to Ulf Hansson

From: Ulf Hansson <[email protected]>

This reverts commit 2fb30d1147c599f5657e8c62c862f9a0f58d9d99.

---
 drivers/spi/spi-pl022.c |    4 ----
 1 file changed, 4 deletions(-)
drivers/spi/spi-pl022.c +0 -4
index 3f2f36c..05cfb61e 100644
@@ -2188,9 +2188,6 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
 	printk(KERN_INFO "pl022: mapped registers from 0x%08x to %p\n",
 	       adev->res.start, pl022->virtbase);
 
-	pm_runtime_enable(dev);
-	pm_runtime_resume(dev);
-
 	pl022->clk = clk_get(&adev->dev, NULL);
 	if (IS_ERR(pl022->clk)) {
 		status = PTR_ERR(pl022->clk);
@@ -2303,7 +2300,6 @@ pl022_remove(struct amba_device *adev)
 	clk_disable(pl022->clk);
 	clk_unprepare(pl022->clk);
 	clk_put(pl022->clk);
-	pm_runtime_disable(&adev->dev);
 	iounmap(pl022->virtbase);
 	amba_release_regions(adev);
 	tasklet_disable(&pl022->pump_transfers);
Signature · 1 line
1.7.10
#11 lore raw

PATCH 2/3 Revert "spi/pl022: enable runtime PM"

Hello ulf,

Quoted 24 lines · Ulf HANSSON

From: Ulf Hansson<[email protected]>

This reverts commit 2fb30d1147c599f5657e8c62c862f9a0f58d9d99.

---
  drivers/spi/spi-pl022.c |    4 ----
  1 file changed, 4 deletions(-)
drivers/spi/spi-pl022.c +0 -3
index 3f2f36c..05cfb61e 100644
@@ -2188,9 +2188,6 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
  	printk(KERN_INFO "pl022: mapped registers from 0x%08x to %p\n",
  	       adev->res.start, pl022->virtbase);
 
-	pm_runtime_enable(dev);
-	pm_runtime_resume(dev);
-
  	pl022->clk = clk_get(&adev->dev, NULL);
  	if (IS_ERR(pl022->clk)) {
  		status = PTR_ERR(pl022->clk);

drivers/spi/spi-pl022.c pl022_remove(struct amba_device *adev) · line 2305

Quoted 7 lines
drivers/spi/spi-pl022.c +0 -1
@@ -2303,7 +2300,6 @@ pl022_remove(struct amba_device *adev)
  	clk_disable(pl022->clk);
  	clk_unprepare(pl022->clk);
  	clk_put(pl022->clk);
-	pm_runtime_disable(&adev->dev);
  	iounmap(pl022->virtbase);
  	amba_release_regions(adev);
  	tasklet_disable(&pl022->pump_transfers);

sorry..Now i got it you revert complete rumtime patch.

Regards
Vipul Samar

#13 lore raw

PATCH 3/3 spi: spi-pl022: Minor simplification for runtime pm

In reply to Ulf Hansson

From: Ulf Hansson <[email protected]>

In probe pm_runtime_put_autosuspend has the same effect as doing pm_runtime_put. This due to upper layer in driver core is preventing the device from being runtime suspended by a pm_runtime_get*.

---
 drivers/spi/spi-pl022.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/spi/spi-pl022.c +2 -3
index 05cfb61e..e51a026 100644
@@ -2248,10 +2248,9 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
 		pm_runtime_set_autosuspend_delay(dev,
 			platform_info->autosuspend_delay);
 		pm_runtime_use_autosuspend(dev);
-		pm_runtime_put_autosuspend(dev);
-	} else {
-		pm_runtime_put(dev);
 	}
+	pm_runtime_put(dev);
+
 	return 0;
 
  err_spi_register:
Signature · 1 line
1.7.10
#15 lore raw

cover Fixup use of runtime pm

In reply to Ulf Hansson

Sorry for the heading, should be spi: spi-pl022"

Kind regards
Ulf Hansson

Quoted 17 lines · Ulf Hansson

From: Ulf Hansson <[email protected]>

Some old runtime pm patches got merged whiched messed up things. These is now reverted. Additionaly one patch do a simplification of the use of runtime pm functions.

Ulf Hansson (3):
Revert "spi/pl022: fix spi-pl022 pm enable at probe" Revert "spi/pl022: enable runtime PM"
spi: spi-pl022: Minor simplification for runtime pm

 drivers/spi/spi-pl022.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

--
1.7.10