Skip to contentSkip to conversation

spi/pl022: Activate resourses before deactivate them in suspend

PATCH New Ulf Hansson 5 participants 8 messages Oct 5, 2012 – Oct 30, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 spi/pl022: Activate resourses before deactivate them in suspend

From: Ulf Hansson <[email protected]>

To be able to deactivate resourses in suspend, the resourses must first be surely active. This is done with a pm_runtime_get_sync. Once the resourses are restored to active state again in resume, the runtime pm usage count can be decreased with a pm_runtime_put.

---
 drivers/spi/spi-pl022.c |    3 +++
 1 file changed, 3 insertions(+)
drivers/spi/spi-pl022.c +3 -0
index 9194641..c3590e0 100644
@@ -2350,6 +2350,8 @@ static int pl022_suspend(struct device *dev)
 		dev_warn(dev, "cannot suspend master\n");
 		return ret;
 	}
+
+	pm_runtime_get_sync(dev);
 	pl022_suspend_resources(pl022);
 
 	dev_dbg(dev, "suspended\n");
@@ -2362,6 +2364,7 @@ static int pl022_resume(struct device *dev)
 	int ret;
 
 	pl022_resume_resources(pl022);
+	pm_runtime_put(dev);
 
 	/* Start the queue running */
 	ret = spi_master_resume(pl022->master);
Signature · 1 line
1.7.10
#1 lore raw

PATCH 1/1 spi/pl022: Activate resourses before deactivate them in suspend

Hi Mark,

Just a kind remember on this. Do you see any problem merging this?

Kind regards
Ulf Hansson

Quoted 36 lines · Ulf Hansson

From: Ulf Hansson <[email protected]>

To be able to deactivate resourses in suspend, the resourses must first be surely active. This is done with a pm_runtime_get_sync. Once the resourses are restored to active state again in resume, the runtime pm usage count can be decreased with a pm_runtime_put.

---
 drivers/spi/spi-pl022.c |    3 +++
 1 file changed, 3 insertions(+)
drivers/spi/spi-pl022.c +3 -1
index 9194641..c3590e0 100644
@@ -2350,6 +2350,8 @@ static int pl022_suspend(struct device *dev)
                dev_warn(dev, "cannot suspend master\n");
                return ret;
        }
+
+       pm_runtime_get_sync(dev);
        pl022_suspend_resources(pl022);
 
        dev_dbg(dev, "suspended\n");
@@ -2362,6 +2364,7 @@ static int pl022_resume(struct device *dev)
        int ret;
 
        pl022_resume_resources(pl022);
+       pm_runtime_put(dev);
 
        /* Start the queue running */
        ret = spi_master_resume(pl022->master);
--

1.7.10

#2 lore raw

PATCH 1/1 spi/pl022: Activate resourses before deactivate them in suspend

Quoted 8 lines · Ulf Hansson

Hi Mark,

Just a kind remember on this. Do you see any problem merging this?

Kind regards
Ulf Hansson

On 5 October 2012 09:43, Ulf Hansson <[email protected]> wrote:

Don't top post and don't send contentless nags less than a week after your original mail, especially not in the merge window when only urgent bug fixes should be applied.

#3 lore raw

PATCH 1/1 spi/pl022: Activate resourses before deactivate them in suspend

In reply to Ulf Hansson
Quoted 4 lines · Ulf Hansson

To be able to deactivate resourses in suspend, the resourses must first be surely active. This is done with a pm_runtime_get_sync. Once the resourses are restored to active state again in resume, the runtime pm usage count can be decreased with a pm_runtime_put.

The PM core will ensure devices are runtime resumed before we enter
suspend precisely due to this sort of issue.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121027/e3ecc64f/attachment.sig>

#4 lore raw

PATCH 1/1 spi/pl022: Activate resourses before deactivate them in suspend

Quoted 9 lines · Mark Brown
Quoted 4 lines · Ulf Hansson

To be able to deactivate resourses in suspend, the resourses must first be surely active. This is done with a pm_runtime_get_sync. Once the resourses are restored to active state again in resume, the runtime pm usage count can be decreased with a pm_runtime_put.

The PM core will ensure devices are runtime resumed before we enter suspend precisely due to this sort of issue.

I asked the very same question to Ulf (in speech, sorry so you couldn't see it...)

So I guess we are talking about drivers/base/main.c

in device_prepare()
pm_runtime_get_noresume() is called
and in device_complete()
pm_runtime_put_sync() is called.

Both put into current for in
commit 88d26136a256576e444db312179e17af6dd0ea87
on sep 19th.

Yes it seems like it will do the job.

Ulf can you comment on this...

Yours,
Linus Walleij

#5 lore raw

PATCH 1/1 spi/pl022: Activate resourses before deactivate them in suspend

Quoted 12 lines · Linus Walleij
Quoted 9 lines · Mark Brown
Quoted 4 lines · Ulf Hansson

To be able to deactivate resourses in suspend, the resourses must first be surely active. This is done with a pm_runtime_get_sync. Once the resourses are restored to active state again in resume, the runtime pm usage count can be decreased with a pm_runtime_put.

The PM core will ensure devices are runtime resumed before we enter suspend precisely due to this sort of issue.

Quoted 7 lines

I asked the very same question to Ulf (in speech, sorry so you couldn't see it...)

So I guess we are talking about drivers/base/main.c

in device_prepare()
pm_runtime_get_noresume() is called

This will increase the "usage_counter" for the device. It will not "runtime_resume" the device, though it will prevent it from being "runtime_suspended".

Quoted 8 lines

and in device_complete()
pm_runtime_put_sync() is called.

Both put into current for in
commit 88d26136a256576e444db312179e17af6dd0ea87
on sep 19th.

Quoted 6 lines

Yes it seems like it will do the job.

Ulf can you comment on this...

Yours,
Linus Walleij

Kind regards
Ulf Hansson

#6 lore raw

PATCH 1/1 spi/pl022: Activate resourses before deactivate them in suspend

Quoted 16 lines · Ulf Hansson
Quoted 15 lines · Linus Walleij
Quoted 9 lines · Mark Brown
Quoted 4 lines · Ulf Hansson

To be able to deactivate resourses in suspend, the resourses must first be surely active. This is done with a pm_runtime_get_sync. Once the resourses are restored to active state again in resume, the runtime pm usage count can be decreased with a pm_runtime_put.

The PM core will ensure devices are runtime resumed before we enter suspend precisely due to this sort of issue.

I asked the very same question to Ulf (in speech, sorry so you couldn't see it...)

Quoted 8 lines
Quoted 4 lines

So I guess we are talking about drivers/base/main.c

in device_prepare()
pm_runtime_get_noresume() is called

This will increase the "usage_counter" for the device. It will not "runtime_resume" the device, though it will prevent it from being "runtime_suspended".

Indeed. The PM core does _not_ insure that devices are runtime resumed before going into system suspend. Some subsystems may do this (the PCI subsystem does, for example -- see drivers/pci/pci-driver.c:pci_pm_prepare()), but the PM core doesn't.

Alan Stern

#7 lore raw

PATCH 1/1 spi/pl022: Activate resourses before deactivate them in suspend

Quoted 27 lines · Alan Stern
Quoted 16 lines · Ulf Hansson
Quoted 15 lines · Linus Walleij
Quoted 9 lines · Mark Brown
Quoted 4 lines · Ulf Hansson

To be able to deactivate resourses in suspend, the resourses must first be surely active. This is done with a pm_runtime_get_sync. Once the resourses are restored to active state again in resume, the runtime pm usage count can be decreased with a pm_runtime_put.

The PM core will ensure devices are runtime resumed before we enter suspend precisely due to this sort of issue.

I asked the very same question to Ulf (in speech, sorry so you couldn't see it...)

Quoted 8 lines
Quoted 4 lines

So I guess we are talking about drivers/base/main.c

in device_prepare()
pm_runtime_get_noresume() is called

This will increase the "usage_counter" for the device. It will not "runtime_resume" the device, though it will prevent it from being "runtime_suspended".

Quoted 7 lines

Indeed. The PM core does _not_ insure that devices are runtime resumed before going into system suspend. Some subsystems may do this (the PCI subsystem does, for example -- see drivers/pci/pci-driver.c:pci_pm_prepare()), but the PM core doesn't.

Alan Stern

Thanks for clarifying this Alan. Although, I am having second thoughts around this patch even if it does what I expect it to do.

I think that the pm_runtime_get_sync (and the corrsponding "put") should maybe be done in the suspend|resume functions of the amba bus (drivers/amba/bus.c) instead.
The reason is because the amba bus is responsible for the apb_pclk - clk, which also should be handled during suspend and this is not the case right now. Unless we think each amba device driver should handle this clock during suspend|resume...

Kind regards
Ulf Hansson