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

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

[Ulf Hansson]

From: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

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.

Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-pl022.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 9194641..c3590e0 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -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


------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev

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

[Ulf Hansson]

From: Ulf Hansson <ulf.hansson@linaro.org>

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.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/spi/spi-pl022.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 9194641..c3590e0 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -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

Re: [PATCH] spi/pl022: Activate resourses before deactivate them in suspend

[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 <ulf.hansson-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org> wrote:
> From: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> 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.
>
> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/spi/spi-pl022.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
> index 9194641..c3590e0 100644
> --- a/drivers/spi/spi-pl022.c
> +++ b/drivers/spi/spi-pl022.c
> @@ -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
>

------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev

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

[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 <ulf.hansson@stericsson.com> wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> 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.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/spi/spi-pl022.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
> index 9194641..c3590e0 100644
> --- a/drivers/spi/spi-pl022.c
> +++ b/drivers/spi/spi-pl022.c
> @@ -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
>

Re: [PATCH] spi/pl022: Activate resourses before deactivate them in suspend

[Mark Brown]

On Fri, Oct 12, 2012 at 04:42:53PM +0200, Ulf Hansson wrote:
> 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 <ulf.hansson-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org> 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.

------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev

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

[Mark Brown]

On Fri, Oct 12, 2012 at 04:42:53PM +0200, Ulf Hansson wrote:
> 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 <ulf.hansson@stericsson.com> 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.

Re: [PATCH] spi/pl022: Activate resourses before deactivate them in suspend

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 447 bytes --]

On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:

> 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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Mark Brown]

On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:

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

Re: [PATCH] spi/pl022: Activate resourses before deactivate them in suspend

[Linus Walleij]

On Sat, Oct 27, 2012 at 11:46 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:
>
>> 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

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

[Linus Walleij]

On Sat, Oct 27, 2012 at 11:46 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:
>
>> 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

Re: [PATCH] spi/pl022: Activate resourses before deactivate them in suspend

[Ulf Hansson]

On 28 October 2012 20:52, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Sat, Oct 27, 2012 at 11:46 PM, Mark Brown
> <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> wrote:
>> On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:
>>
>>> 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

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".

> 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

Kind regards
Ulf Hansson

------------------------------------------------------------------------------
WINDOWS 8 is here. 
Millions of people.  Your app in 30 days.
Visit The Windows 8 Center at Sourceforge for all your go to resources.
http://windows8center.sourceforge.net/
join-generation-app-and-make-money-coding-fast/

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

[Ulf Hansson]

On 28 October 2012 20:52, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Oct 27, 2012 at 11:46 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
>> On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:
>>
>>> 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

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".

> 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

Kind regards
Ulf Hansson

Re: [PATCH] spi/pl022: Activate resourses before deactivate them in suspend

[Alan Stern]

On Sun, 28 Oct 2012, Ulf Hansson wrote:

> On 28 October 2012 20:52, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Sat, Oct 27, 2012 at 11:46 PM, Mark Brown
> > <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> wrote:
> >> On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:
> >>
> >>> 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
> 
> 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


------------------------------------------------------------------------------
WINDOWS 8 is here. 
Millions of people.  Your app in 30 days.
Visit The Windows 8 Center at Sourceforge for all your go to resources.
http://windows8center.sourceforge.net/
join-generation-app-and-make-money-coding-fast/

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

[Alan Stern]

On Sun, 28 Oct 2012, Ulf Hansson wrote:

> On 28 October 2012 20:52, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Sat, Oct 27, 2012 at 11:46 PM, Mark Brown
> > <broonie@opensource.wolfsonmicro.com> wrote:
> >> On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:
> >>
> >>> 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
> 
> 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

Re: [PATCH] spi/pl022: Activate resourses before deactivate them in suspend

[Ulf Hansson]

On 28 October 2012 22:09, Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org> wrote:
> On Sun, 28 Oct 2012, Ulf Hansson wrote:
>
>> On 28 October 2012 20:52, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > On Sat, Oct 27, 2012 at 11:46 PM, Mark Brown
>> > <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> wrote:
>> >> On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:
>> >>
>> >>> 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
>>
>> 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
>

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

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct

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

[Ulf Hansson]

On 28 October 2012 22:09, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Sun, 28 Oct 2012, Ulf Hansson wrote:
>
>> On 28 October 2012 20:52, Linus Walleij <linus.walleij@linaro.org> wrote:
>> > On Sat, Oct 27, 2012 at 11:46 PM, Mark Brown
>> > <broonie@opensource.wolfsonmicro.com> wrote:
>> >> On Fri, Oct 05, 2012 at 09:43:32AM +0200, Ulf Hansson wrote:
>> >>
>> >>> 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
>>
>> 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
>

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