Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Ulf Hansson
On 10 November 2014 17:36, Alan Stern  wrote:
> On Mon, 10 Nov 2014, Ulf Hansson wrote:
>
>> > To me, this sounds like a good reason to avoid using
>> > force_runtime_suspend().  In fact, it sounds like a good reason to
>> > avoid relying on the runtime PM mechanism to handle non-runtime-PM
>> > things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't
>> > enabled then the runtime PM stack simply should not be used.
>>
>> There are an important advantage of using the pm_runtime_force_suspend() 
>> here.
>>
>> For the driver to handle clock gating at system PM suspend, it first
>> needs to bring the device into full power, through
>> pm_runtime_get_sync(). Otherwise it's not safe to gate the clock,
>> since it may already be gated.
>
> That's fine, but it has nothing to do with pm_runtime_force_suspend().
>
> Besides, if the real question is whether or not to gate the clock (or
> in other words, has the clock already been gated), why not just store a
> "clock_is_gated" flag somewhere?

You could do that, but it's easier to not.

You will need to update the runtime PM status and disable runtime PM
anyway, done by the API.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Alan Stern
On Mon, 10 Nov 2014, Ulf Hansson wrote:

> > To me, this sounds like a good reason to avoid using
> > force_runtime_suspend().  In fact, it sounds like a good reason to
> > avoid relying on the runtime PM mechanism to handle non-runtime-PM
> > things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't
> > enabled then the runtime PM stack simply should not be used.
> 
> There are an important advantage of using the pm_runtime_force_suspend() here.
> 
> For the driver to handle clock gating at system PM suspend, it first
> needs to bring the device into full power, through
> pm_runtime_get_sync(). Otherwise it's not safe to gate the clock,
> since it may already be gated.

That's fine, but it has nothing to do with pm_runtime_force_suspend().

Besides, if the real question is whether or not to gate the clock (or 
in other words, has the clock already been gated), why not just store a 
"clock_is_gated" flag somewhere?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Ulf Hansson
On 7 November 2014 15:50, Alan Stern  wrote:
> On Fri, 7 Nov 2014, Krzysztof Kozlowski wrote:
>
>> > Well, that is a good reason to introduce a wrapper around power.irq_safe 
>> > in my
>> > view.
>> >
>> > And define the wrapper so that it always returns false for 
>> > CONFIG_PM_RUNTIME
>> > unset.
>> >
>> > This way not only you wouldn't need to move the flag from under the #ifdef,
>> > but also you would make the compiler skip the relevant pieces of code
>> > entiretly for CONFIG_PM_RUNTIME unset.
>>
>> Few days ago I would be happy with your opinion :), but know I think
>> this is better solution than wrapper. Consider case:
>> 1. PM_RUNTIME unset.
>> 2. System suspends.
>> 3. The pl330 in its suspend callback calls force_runtime_suspend which
>> leads us to amba/bus.
>> 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
>> so it disables and unprepares the clock.
>> 5. The pl330 in probe requested irq_safe so it assumes amba/bus will
>> only disable the clock. So the pl330 unprepares the clock. Again.
>
> To me, this sounds like a good reason to avoid using
> force_runtime_suspend().  In fact, it sounds like a good reason to
> avoid relying on the runtime PM mechanism to handle non-runtime-PM
> things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't
> enabled then the runtime PM stack simply should not be used.

There are an important advantage of using the pm_runtime_force_suspend() here.

For the driver to handle clock gating at system PM suspend, it first
needs to bring the device into full power, through
pm_runtime_get_sync(). Otherwise it's not safe to gate the clock,
since it may already be gated.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Srikanth K
unsubscribe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Ulf Hansson
On 7 November 2014 09:06, Krzysztof Kozlowski  wrote:
> On czw, 2014-11-06 at 23:51 +0100, Rafael J. Wysocki wrote:
>> On Thursday, November 06, 2014 09:36:46 AM Krzysztof Kozlowski wrote:
>> > Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
>> > called by child driver, regardless of CONFIG_PM_RUNTIME.
>> >
>> > An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
>> > suspend/resume callbacks in amba bus driver act differently if irq_safe
>> > was set by child driver (in irq_safe mode bus clock is only disabled).
>> >
>> > The pl330 driver sets irq_safe and assumes that amba bus driver will
>> > only disable the clock in runtime PM. So in system sleep suspend
>> > callback the pl330 driver unprepares the clock after calling
>> > pm_runtime_force_suspend().
>> >
>> > However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
>> > child drivers do not want the irq_safe runtime PM. In such case amba bus
>> > driver still has to know whether child driver wanted irq_safe - by
>> > looking at dev->power.irq_safe field.
>> >
>> > Signed-off-by: Krzysztof Kozlowski 
>> > ---
>> >  include/linux/pm.h | 2 +-
>> >  include/linux/pm_runtime.h | 5 -
>> >  2 files changed, 5 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/include/linux/pm.h b/include/linux/pm.h
>> > index 383fd68aaee1..b05fa954f50d 100644
>> > --- a/include/linux/pm.h
>> > +++ b/include/linux/pm.h
>> > @@ -566,6 +566,7 @@ struct dev_pm_info {
>> > boolignore_children:1;
>> > boolearly_init:1;   /* Owned by the PM core */
>> > booldirect_complete:1;  /* Owned by the PM 
>> > core */
>> > +   unsigned intirq_safe:1; /* PM runtime */
>> > spinlock_t  lock;
>> >  #ifdef CONFIG_PM_SLEEP
>> > struct list_headentry;
>> > @@ -590,7 +591,6 @@ struct dev_pm_info {
>> > unsigned intrun_wake:1;
>> > unsigned intruntime_auto:1;
>> > unsigned intno_callbacks:1;
>> > -   unsigned intirq_safe:1;
>> > unsigned intuse_autosuspend:1;
>> > unsigned inttimer_autosuspends:1;
>> > unsigned intmemalloc_noio:1;
>>
>> Well, that is a good reason to introduce a wrapper around power.irq_safe in 
>> my
>> view.
>>
>> And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
>> unset.
>>
>> This way not only you wouldn't need to move the flag from under the #ifdef,
>> but also you would make the compiler skip the relevant pieces of code
>> entiretly for CONFIG_PM_RUNTIME unset.
>
> Few days ago I would be happy with your opinion :), but know I think
> this is better solution than wrapper. Consider case:
> 1. PM_RUNTIME unset.
> 2. System suspends.
> 3. The pl330 in its suspend callback calls force_runtime_suspend which
> leads us to amba/bus.
> 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
> so it disables and unprepares the clock.
> 5. The pl330 in probe requested irq_safe so it assumes amba/bus will
> only disable the clock. So the pl330 unprepares the clock. Again.

This is easy to solve, still by using Rafael's approach.

In the pl330 system PM callbacks, you need to check
"pm_runtime_is_irqsafe()" or whatever the wrapper would be called.
When it returns true, that's when you should do
clk_prepare|unprepare().

I think that would be quite nice.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Ulf Hansson
On 7 November 2014 09:06, Krzysztof Kozlowski k.kozlow...@samsung.com wrote:
 On czw, 2014-11-06 at 23:51 +0100, Rafael J. Wysocki wrote:
 On Thursday, November 06, 2014 09:36:46 AM Krzysztof Kozlowski wrote:
  Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
  called by child driver, regardless of CONFIG_PM_RUNTIME.
 
  An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
  suspend/resume callbacks in amba bus driver act differently if irq_safe
  was set by child driver (in irq_safe mode bus clock is only disabled).
 
  The pl330 driver sets irq_safe and assumes that amba bus driver will
  only disable the clock in runtime PM. So in system sleep suspend
  callback the pl330 driver unprepares the clock after calling
  pm_runtime_force_suspend().
 
  However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
  child drivers do not want the irq_safe runtime PM. In such case amba bus
  driver still has to know whether child driver wanted irq_safe - by
  looking at dev-power.irq_safe field.
 
  Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com
  ---
   include/linux/pm.h | 2 +-
   include/linux/pm_runtime.h | 5 -
   2 files changed, 5 insertions(+), 2 deletions(-)
 
  diff --git a/include/linux/pm.h b/include/linux/pm.h
  index 383fd68aaee1..b05fa954f50d 100644
  --- a/include/linux/pm.h
  +++ b/include/linux/pm.h
  @@ -566,6 +566,7 @@ struct dev_pm_info {
  boolignore_children:1;
  boolearly_init:1;   /* Owned by the PM core */
  booldirect_complete:1;  /* Owned by the PM 
  core */
  +   unsigned intirq_safe:1; /* PM runtime */
  spinlock_t  lock;
   #ifdef CONFIG_PM_SLEEP
  struct list_headentry;
  @@ -590,7 +591,6 @@ struct dev_pm_info {
  unsigned intrun_wake:1;
  unsigned intruntime_auto:1;
  unsigned intno_callbacks:1;
  -   unsigned intirq_safe:1;
  unsigned intuse_autosuspend:1;
  unsigned inttimer_autosuspends:1;
  unsigned intmemalloc_noio:1;

 Well, that is a good reason to introduce a wrapper around power.irq_safe in 
 my
 view.

 And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
 unset.

 This way not only you wouldn't need to move the flag from under the #ifdef,
 but also you would make the compiler skip the relevant pieces of code
 entiretly for CONFIG_PM_RUNTIME unset.

 Few days ago I would be happy with your opinion :), but know I think
 this is better solution than wrapper. Consider case:
 1. PM_RUNTIME unset.
 2. System suspends.
 3. The pl330 in its suspend callback calls force_runtime_suspend which
 leads us to amba/bus.
 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
 so it disables and unprepares the clock.
 5. The pl330 in probe requested irq_safe so it assumes amba/bus will
 only disable the clock. So the pl330 unprepares the clock. Again.

This is easy to solve, still by using Rafael's approach.

In the pl330 system PM callbacks, you need to check
pm_runtime_is_irqsafe() or whatever the wrapper would be called.
When it returns true, that's when you should do
clk_prepare|unprepare().

I think that would be quite nice.

Kind regards
Uffe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Srikanth K
unsubscribe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Ulf Hansson
On 7 November 2014 15:50, Alan Stern st...@rowland.harvard.edu wrote:
 On Fri, 7 Nov 2014, Krzysztof Kozlowski wrote:

  Well, that is a good reason to introduce a wrapper around power.irq_safe 
  in my
  view.
 
  And define the wrapper so that it always returns false for 
  CONFIG_PM_RUNTIME
  unset.
 
  This way not only you wouldn't need to move the flag from under the #ifdef,
  but also you would make the compiler skip the relevant pieces of code
  entiretly for CONFIG_PM_RUNTIME unset.

 Few days ago I would be happy with your opinion :), but know I think
 this is better solution than wrapper. Consider case:
 1. PM_RUNTIME unset.
 2. System suspends.
 3. The pl330 in its suspend callback calls force_runtime_suspend which
 leads us to amba/bus.
 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
 so it disables and unprepares the clock.
 5. The pl330 in probe requested irq_safe so it assumes amba/bus will
 only disable the clock. So the pl330 unprepares the clock. Again.

 To me, this sounds like a good reason to avoid using
 force_runtime_suspend().  In fact, it sounds like a good reason to
 avoid relying on the runtime PM mechanism to handle non-runtime-PM
 things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't
 enabled then the runtime PM stack simply should not be used.

There are an important advantage of using the pm_runtime_force_suspend() here.

For the driver to handle clock gating at system PM suspend, it first
needs to bring the device into full power, through
pm_runtime_get_sync(). Otherwise it's not safe to gate the clock,
since it may already be gated.

Kind regards
Uffe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Alan Stern
On Mon, 10 Nov 2014, Ulf Hansson wrote:

  To me, this sounds like a good reason to avoid using
  force_runtime_suspend().  In fact, it sounds like a good reason to
  avoid relying on the runtime PM mechanism to handle non-runtime-PM
  things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't
  enabled then the runtime PM stack simply should not be used.
 
 There are an important advantage of using the pm_runtime_force_suspend() here.
 
 For the driver to handle clock gating at system PM suspend, it first
 needs to bring the device into full power, through
 pm_runtime_get_sync(). Otherwise it's not safe to gate the clock,
 since it may already be gated.

That's fine, but it has nothing to do with pm_runtime_force_suspend().

Besides, if the real question is whether or not to gate the clock (or 
in other words, has the clock already been gated), why not just store a 
clock_is_gated flag somewhere?

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-10 Thread Ulf Hansson
On 10 November 2014 17:36, Alan Stern st...@rowland.harvard.edu wrote:
 On Mon, 10 Nov 2014, Ulf Hansson wrote:

  To me, this sounds like a good reason to avoid using
  force_runtime_suspend().  In fact, it sounds like a good reason to
  avoid relying on the runtime PM mechanism to handle non-runtime-PM
  things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't
  enabled then the runtime PM stack simply should not be used.

 There are an important advantage of using the pm_runtime_force_suspend() 
 here.

 For the driver to handle clock gating at system PM suspend, it first
 needs to bring the device into full power, through
 pm_runtime_get_sync(). Otherwise it's not safe to gate the clock,
 since it may already be gated.

 That's fine, but it has nothing to do with pm_runtime_force_suspend().

 Besides, if the real question is whether or not to gate the clock (or
 in other words, has the clock already been gated), why not just store a
 clock_is_gated flag somewhere?

You could do that, but it's easier to not.

You will need to update the runtime PM status and disable runtime PM
anyway, done by the API.

Kind regards
Uffe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-07 Thread Rafael J. Wysocki
On Friday, November 07, 2014 09:50:58 AM Alan Stern wrote:
> On Fri, 7 Nov 2014, Krzysztof Kozlowski wrote:
> 
> > > Well, that is a good reason to introduce a wrapper around power.irq_safe 
> > > in my
> > > view.
> > > 
> > > And define the wrapper so that it always returns false for 
> > > CONFIG_PM_RUNTIME
> > > unset.
> > > 
> > > This way not only you wouldn't need to move the flag from under the 
> > > #ifdef,
> > > but also you would make the compiler skip the relevant pieces of code
> > > entiretly for CONFIG_PM_RUNTIME unset.
> > 
> > Few days ago I would be happy with your opinion :), but know I think
> > this is better solution than wrapper. Consider case:
> > 1. PM_RUNTIME unset.
> > 2. System suspends.
> > 3. The pl330 in its suspend callback calls force_runtime_suspend which
> > leads us to amba/bus.
> > 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
> > so it disables and unprepares the clock.
> > 5. The pl330 in probe requested irq_safe so it assumes amba/bus will
> > only disable the clock. So the pl330 unprepares the clock. Again.
> 
> To me, this sounds like a good reason to avoid using 
> force_runtime_suspend().  In fact, it sounds like a good reason to 
> avoid relying on the runtime PM mechanism to handle non-runtime-PM 
> things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't 
> enabled then the runtime PM stack simply should not be used.

Amen.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-07 Thread Alan Stern
On Fri, 7 Nov 2014, Krzysztof Kozlowski wrote:

> > Well, that is a good reason to introduce a wrapper around power.irq_safe in 
> > my
> > view.
> > 
> > And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
> > unset.
> > 
> > This way not only you wouldn't need to move the flag from under the #ifdef,
> > but also you would make the compiler skip the relevant pieces of code
> > entiretly for CONFIG_PM_RUNTIME unset.
> 
> Few days ago I would be happy with your opinion :), but know I think
> this is better solution than wrapper. Consider case:
> 1. PM_RUNTIME unset.
> 2. System suspends.
> 3. The pl330 in its suspend callback calls force_runtime_suspend which
> leads us to amba/bus.
> 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
> so it disables and unprepares the clock.
> 5. The pl330 in probe requested irq_safe so it assumes amba/bus will
> only disable the clock. So the pl330 unprepares the clock. Again.

To me, this sounds like a good reason to avoid using 
force_runtime_suspend().  In fact, it sounds like a good reason to 
avoid relying on the runtime PM mechanism to handle non-runtime-PM 
things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't 
enabled then the runtime PM stack simply should not be used.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-07 Thread Krzysztof Kozlowski
On czw, 2014-11-06 at 23:51 +0100, Rafael J. Wysocki wrote:
> On Thursday, November 06, 2014 09:36:46 AM Krzysztof Kozlowski wrote:
> > Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
> > called by child driver, regardless of CONFIG_PM_RUNTIME.
> > 
> > An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
> > suspend/resume callbacks in amba bus driver act differently if irq_safe
> > was set by child driver (in irq_safe mode bus clock is only disabled).
> > 
> > The pl330 driver sets irq_safe and assumes that amba bus driver will
> > only disable the clock in runtime PM. So in system sleep suspend
> > callback the pl330 driver unprepares the clock after calling
> > pm_runtime_force_suspend().
> > 
> > However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
> > child drivers do not want the irq_safe runtime PM. In such case amba bus
> > driver still has to know whether child driver wanted irq_safe - by
> > looking at dev->power.irq_safe field.
> > 
> > Signed-off-by: Krzysztof Kozlowski 
> > ---
> >  include/linux/pm.h | 2 +-
> >  include/linux/pm_runtime.h | 5 -
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/pm.h b/include/linux/pm.h
> > index 383fd68aaee1..b05fa954f50d 100644
> > --- a/include/linux/pm.h
> > +++ b/include/linux/pm.h
> > @@ -566,6 +566,7 @@ struct dev_pm_info {
> > boolignore_children:1;
> > boolearly_init:1;   /* Owned by the PM core */
> > booldirect_complete:1;  /* Owned by the PM core 
> > */
> > +   unsigned intirq_safe:1; /* PM runtime */
> > spinlock_t  lock;
> >  #ifdef CONFIG_PM_SLEEP
> > struct list_headentry;
> > @@ -590,7 +591,6 @@ struct dev_pm_info {
> > unsigned intrun_wake:1;
> > unsigned intruntime_auto:1;
> > unsigned intno_callbacks:1;
> > -   unsigned intirq_safe:1;
> > unsigned intuse_autosuspend:1;
> > unsigned inttimer_autosuspends:1;
> > unsigned intmemalloc_noio:1;
> 
> Well, that is a good reason to introduce a wrapper around power.irq_safe in my
> view.
> 
> And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
> unset.
> 
> This way not only you wouldn't need to move the flag from under the #ifdef,
> but also you would make the compiler skip the relevant pieces of code
> entiretly for CONFIG_PM_RUNTIME unset.

Few days ago I would be happy with your opinion :), but know I think
this is better solution than wrapper. Consider case:
1. PM_RUNTIME unset.
2. System suspends.
3. The pl330 in its suspend callback calls force_runtime_suspend which
leads us to amba/bus.
4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
so it disables and unprepares the clock.
5. The pl330 in probe requested irq_safe so it assumes amba/bus will
only disable the clock. So the pl330 unprepares the clock. Again.

Best regards,
Krzysztof


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-07 Thread Krzysztof Kozlowski
On czw, 2014-11-06 at 23:51 +0100, Rafael J. Wysocki wrote:
 On Thursday, November 06, 2014 09:36:46 AM Krzysztof Kozlowski wrote:
  Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
  called by child driver, regardless of CONFIG_PM_RUNTIME.
  
  An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
  suspend/resume callbacks in amba bus driver act differently if irq_safe
  was set by child driver (in irq_safe mode bus clock is only disabled).
  
  The pl330 driver sets irq_safe and assumes that amba bus driver will
  only disable the clock in runtime PM. So in system sleep suspend
  callback the pl330 driver unprepares the clock after calling
  pm_runtime_force_suspend().
  
  However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
  child drivers do not want the irq_safe runtime PM. In such case amba bus
  driver still has to know whether child driver wanted irq_safe - by
  looking at dev-power.irq_safe field.
  
  Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com
  ---
   include/linux/pm.h | 2 +-
   include/linux/pm_runtime.h | 5 -
   2 files changed, 5 insertions(+), 2 deletions(-)
  
  diff --git a/include/linux/pm.h b/include/linux/pm.h
  index 383fd68aaee1..b05fa954f50d 100644
  --- a/include/linux/pm.h
  +++ b/include/linux/pm.h
  @@ -566,6 +566,7 @@ struct dev_pm_info {
  boolignore_children:1;
  boolearly_init:1;   /* Owned by the PM core */
  booldirect_complete:1;  /* Owned by the PM core 
  */
  +   unsigned intirq_safe:1; /* PM runtime */
  spinlock_t  lock;
   #ifdef CONFIG_PM_SLEEP
  struct list_headentry;
  @@ -590,7 +591,6 @@ struct dev_pm_info {
  unsigned intrun_wake:1;
  unsigned intruntime_auto:1;
  unsigned intno_callbacks:1;
  -   unsigned intirq_safe:1;
  unsigned intuse_autosuspend:1;
  unsigned inttimer_autosuspends:1;
  unsigned intmemalloc_noio:1;
 
 Well, that is a good reason to introduce a wrapper around power.irq_safe in my
 view.
 
 And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
 unset.
 
 This way not only you wouldn't need to move the flag from under the #ifdef,
 but also you would make the compiler skip the relevant pieces of code
 entiretly for CONFIG_PM_RUNTIME unset.

Few days ago I would be happy with your opinion :), but know I think
this is better solution than wrapper. Consider case:
1. PM_RUNTIME unset.
2. System suspends.
3. The pl330 in its suspend callback calls force_runtime_suspend which
leads us to amba/bus.
4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
so it disables and unprepares the clock.
5. The pl330 in probe requested irq_safe so it assumes amba/bus will
only disable the clock. So the pl330 unprepares the clock. Again.

Best regards,
Krzysztof


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-07 Thread Alan Stern
On Fri, 7 Nov 2014, Krzysztof Kozlowski wrote:

  Well, that is a good reason to introduce a wrapper around power.irq_safe in 
  my
  view.
  
  And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
  unset.
  
  This way not only you wouldn't need to move the flag from under the #ifdef,
  but also you would make the compiler skip the relevant pieces of code
  entiretly for CONFIG_PM_RUNTIME unset.
 
 Few days ago I would be happy with your opinion :), but know I think
 this is better solution than wrapper. Consider case:
 1. PM_RUNTIME unset.
 2. System suspends.
 3. The pl330 in its suspend callback calls force_runtime_suspend which
 leads us to amba/bus.
 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
 so it disables and unprepares the clock.
 5. The pl330 in probe requested irq_safe so it assumes amba/bus will
 only disable the clock. So the pl330 unprepares the clock. Again.

To me, this sounds like a good reason to avoid using 
force_runtime_suspend().  In fact, it sounds like a good reason to 
avoid relying on the runtime PM mechanism to handle non-runtime-PM 
things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't 
enabled then the runtime PM stack simply should not be used.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-07 Thread Rafael J. Wysocki
On Friday, November 07, 2014 09:50:58 AM Alan Stern wrote:
 On Fri, 7 Nov 2014, Krzysztof Kozlowski wrote:
 
   Well, that is a good reason to introduce a wrapper around power.irq_safe 
   in my
   view.
   
   And define the wrapper so that it always returns false for 
   CONFIG_PM_RUNTIME
   unset.
   
   This way not only you wouldn't need to move the flag from under the 
   #ifdef,
   but also you would make the compiler skip the relevant pieces of code
   entiretly for CONFIG_PM_RUNTIME unset.
  
  Few days ago I would be happy with your opinion :), but know I think
  this is better solution than wrapper. Consider case:
  1. PM_RUNTIME unset.
  2. System suspends.
  3. The pl330 in its suspend callback calls force_runtime_suspend which
  leads us to amba/bus.
  4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE),
  so it disables and unprepares the clock.
  5. The pl330 in probe requested irq_safe so it assumes amba/bus will
  only disable the clock. So the pl330 unprepares the clock. Again.
 
 To me, this sounds like a good reason to avoid using 
 force_runtime_suspend().  In fact, it sounds like a good reason to 
 avoid relying on the runtime PM mechanism to handle non-runtime-PM 
 things (like a system suspend callback).  If CONFIG_PM_RUNTIME isn't 
 enabled then the runtime PM stack simply should not be used.

Amen.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-06 Thread Rafael J. Wysocki
On Thursday, November 06, 2014 09:36:46 AM Krzysztof Kozlowski wrote:
> Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
> called by child driver, regardless of CONFIG_PM_RUNTIME.
> 
> An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
> suspend/resume callbacks in amba bus driver act differently if irq_safe
> was set by child driver (in irq_safe mode bus clock is only disabled).
> 
> The pl330 driver sets irq_safe and assumes that amba bus driver will
> only disable the clock in runtime PM. So in system sleep suspend
> callback the pl330 driver unprepares the clock after calling
> pm_runtime_force_suspend().
> 
> However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
> child drivers do not want the irq_safe runtime PM. In such case amba bus
> driver still has to know whether child driver wanted irq_safe - by
> looking at dev->power.irq_safe field.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  include/linux/pm.h | 2 +-
>  include/linux/pm_runtime.h | 5 -
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 383fd68aaee1..b05fa954f50d 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -566,6 +566,7 @@ struct dev_pm_info {
>   boolignore_children:1;
>   boolearly_init:1;   /* Owned by the PM core */
>   booldirect_complete:1;  /* Owned by the PM core 
> */
> + unsigned intirq_safe:1; /* PM runtime */
>   spinlock_t  lock;
>  #ifdef CONFIG_PM_SLEEP
>   struct list_headentry;
> @@ -590,7 +591,6 @@ struct dev_pm_info {
>   unsigned intrun_wake:1;
>   unsigned intruntime_auto:1;
>   unsigned intno_callbacks:1;
> - unsigned intirq_safe:1;
>   unsigned intuse_autosuspend:1;
>   unsigned inttimer_autosuspends:1;
>   unsigned intmemalloc_noio:1;

Well, that is a good reason to introduce a wrapper around power.irq_safe in my
view.

And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
unset.

This way not only you wouldn't need to move the flag from under the #ifdef,
but also you would make the compiler skip the relevant pieces of code
entiretly for CONFIG_PM_RUNTIME unset.

> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index 367f49b9a1c9..d94a65662a60 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -166,7 +166,10 @@ static inline bool 
> pm_runtime_suspended_if_enabled(struct device *dev) { return
>  static inline bool pm_runtime_enabled(struct device *dev) { return false; }
>  
>  static inline void pm_runtime_no_callbacks(struct device *dev) {}
> -static inline void pm_runtime_irq_safe(struct device *dev) {}
> +static inline void pm_runtime_irq_safe(struct device *dev)
> +{
> + dev->power.irq_safe = 1;
> +}
>  
>  static inline bool pm_runtime_callbacks_present(struct device *dev) { return 
> false; }
>  static inline void pm_runtime_mark_last_busy(struct device *dev) {}
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-06 Thread Ulf Hansson
On 6 November 2014 09:36, Krzysztof Kozlowski  wrote:
> Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
> called by child driver, regardless of CONFIG_PM_RUNTIME.
>
> An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
> suspend/resume callbacks in amba bus driver act differently if irq_safe
> was set by child driver (in irq_safe mode bus clock is only disabled).
>
> The pl330 driver sets irq_safe and assumes that amba bus driver will
> only disable the clock in runtime PM. So in system sleep suspend
> callback the pl330 driver unprepares the clock after calling
> pm_runtime_force_suspend().
>
> However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
> child drivers do not want the irq_safe runtime PM. In such case amba bus
> driver still has to know whether child driver wanted irq_safe - by
> looking at dev->power.irq_safe field.
>
> Signed-off-by: Krzysztof Kozlowski 

FWIW: Reviewed-by: Ulf Hansson 

Kind regards
Uffe

> ---
>  include/linux/pm.h | 2 +-
>  include/linux/pm_runtime.h | 5 -
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 383fd68aaee1..b05fa954f50d 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -566,6 +566,7 @@ struct dev_pm_info {
> boolignore_children:1;
> boolearly_init:1;   /* Owned by the PM core */
> booldirect_complete:1;  /* Owned by the PM 
> core */
> +   unsigned intirq_safe:1; /* PM runtime */
> spinlock_t  lock;
>  #ifdef CONFIG_PM_SLEEP
> struct list_headentry;
> @@ -590,7 +591,6 @@ struct dev_pm_info {
> unsigned intrun_wake:1;
> unsigned intruntime_auto:1;
> unsigned intno_callbacks:1;
> -   unsigned intirq_safe:1;
> unsigned intuse_autosuspend:1;
> unsigned inttimer_autosuspends:1;
> unsigned intmemalloc_noio:1;
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index 367f49b9a1c9..d94a65662a60 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -166,7 +166,10 @@ static inline bool 
> pm_runtime_suspended_if_enabled(struct device *dev) { return
>  static inline bool pm_runtime_enabled(struct device *dev) { return false; }
>
>  static inline void pm_runtime_no_callbacks(struct device *dev) {}
> -static inline void pm_runtime_irq_safe(struct device *dev) {}
> +static inline void pm_runtime_irq_safe(struct device *dev)
> +{
> +   dev->power.irq_safe = 1;
> +}
>
>  static inline bool pm_runtime_callbacks_present(struct device *dev) { return 
> false; }
>  static inline void pm_runtime_mark_last_busy(struct device *dev) {}
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-06 Thread Krzysztof Kozlowski
Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
called by child driver, regardless of CONFIG_PM_RUNTIME.

An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
suspend/resume callbacks in amba bus driver act differently if irq_safe
was set by child driver (in irq_safe mode bus clock is only disabled).

The pl330 driver sets irq_safe and assumes that amba bus driver will
only disable the clock in runtime PM. So in system sleep suspend
callback the pl330 driver unprepares the clock after calling
pm_runtime_force_suspend().

However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
child drivers do not want the irq_safe runtime PM. In such case amba bus
driver still has to know whether child driver wanted irq_safe - by
looking at dev->power.irq_safe field.

Signed-off-by: Krzysztof Kozlowski 
---
 include/linux/pm.h | 2 +-
 include/linux/pm_runtime.h | 5 -
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 383fd68aaee1..b05fa954f50d 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -566,6 +566,7 @@ struct dev_pm_info {
boolignore_children:1;
boolearly_init:1;   /* Owned by the PM core */
booldirect_complete:1;  /* Owned by the PM core 
*/
+   unsigned intirq_safe:1; /* PM runtime */
spinlock_t  lock;
 #ifdef CONFIG_PM_SLEEP
struct list_headentry;
@@ -590,7 +591,6 @@ struct dev_pm_info {
unsigned intrun_wake:1;
unsigned intruntime_auto:1;
unsigned intno_callbacks:1;
-   unsigned intirq_safe:1;
unsigned intuse_autosuspend:1;
unsigned inttimer_autosuspends:1;
unsigned intmemalloc_noio:1;
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 367f49b9a1c9..d94a65662a60 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -166,7 +166,10 @@ static inline bool pm_runtime_suspended_if_enabled(struct 
device *dev) { return
 static inline bool pm_runtime_enabled(struct device *dev) { return false; }
 
 static inline void pm_runtime_no_callbacks(struct device *dev) {}
-static inline void pm_runtime_irq_safe(struct device *dev) {}
+static inline void pm_runtime_irq_safe(struct device *dev)
+{
+   dev->power.irq_safe = 1;
+}
 
 static inline bool pm_runtime_callbacks_present(struct device *dev) { return 
false; }
 static inline void pm_runtime_mark_last_busy(struct device *dev) {}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-06 Thread Rafael J. Wysocki
On Thursday, November 06, 2014 09:36:46 AM Krzysztof Kozlowski wrote:
 Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
 called by child driver, regardless of CONFIG_PM_RUNTIME.
 
 An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
 suspend/resume callbacks in amba bus driver act differently if irq_safe
 was set by child driver (in irq_safe mode bus clock is only disabled).
 
 The pl330 driver sets irq_safe and assumes that amba bus driver will
 only disable the clock in runtime PM. So in system sleep suspend
 callback the pl330 driver unprepares the clock after calling
 pm_runtime_force_suspend().
 
 However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
 child drivers do not want the irq_safe runtime PM. In such case amba bus
 driver still has to know whether child driver wanted irq_safe - by
 looking at dev-power.irq_safe field.
 
 Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com
 ---
  include/linux/pm.h | 2 +-
  include/linux/pm_runtime.h | 5 -
  2 files changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/include/linux/pm.h b/include/linux/pm.h
 index 383fd68aaee1..b05fa954f50d 100644
 --- a/include/linux/pm.h
 +++ b/include/linux/pm.h
 @@ -566,6 +566,7 @@ struct dev_pm_info {
   boolignore_children:1;
   boolearly_init:1;   /* Owned by the PM core */
   booldirect_complete:1;  /* Owned by the PM core 
 */
 + unsigned intirq_safe:1; /* PM runtime */
   spinlock_t  lock;
  #ifdef CONFIG_PM_SLEEP
   struct list_headentry;
 @@ -590,7 +591,6 @@ struct dev_pm_info {
   unsigned intrun_wake:1;
   unsigned intruntime_auto:1;
   unsigned intno_callbacks:1;
 - unsigned intirq_safe:1;
   unsigned intuse_autosuspend:1;
   unsigned inttimer_autosuspends:1;
   unsigned intmemalloc_noio:1;

Well, that is a good reason to introduce a wrapper around power.irq_safe in my
view.

And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME
unset.

This way not only you wouldn't need to move the flag from under the #ifdef,
but also you would make the compiler skip the relevant pieces of code
entiretly for CONFIG_PM_RUNTIME unset.

 diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
 index 367f49b9a1c9..d94a65662a60 100644
 --- a/include/linux/pm_runtime.h
 +++ b/include/linux/pm_runtime.h
 @@ -166,7 +166,10 @@ static inline bool 
 pm_runtime_suspended_if_enabled(struct device *dev) { return
  static inline bool pm_runtime_enabled(struct device *dev) { return false; }
  
  static inline void pm_runtime_no_callbacks(struct device *dev) {}
 -static inline void pm_runtime_irq_safe(struct device *dev) {}
 +static inline void pm_runtime_irq_safe(struct device *dev)
 +{
 + dev-power.irq_safe = 1;
 +}
  
  static inline bool pm_runtime_callbacks_present(struct device *dev) { return 
 false; }
  static inline void pm_runtime_mark_last_busy(struct device *dev) {}
 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-06 Thread Krzysztof Kozlowski
Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
called by child driver, regardless of CONFIG_PM_RUNTIME.

An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
suspend/resume callbacks in amba bus driver act differently if irq_safe
was set by child driver (in irq_safe mode bus clock is only disabled).

The pl330 driver sets irq_safe and assumes that amba bus driver will
only disable the clock in runtime PM. So in system sleep suspend
callback the pl330 driver unprepares the clock after calling
pm_runtime_force_suspend().

However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
child drivers do not want the irq_safe runtime PM. In such case amba bus
driver still has to know whether child driver wanted irq_safe - by
looking at dev-power.irq_safe field.

Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com
---
 include/linux/pm.h | 2 +-
 include/linux/pm_runtime.h | 5 -
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 383fd68aaee1..b05fa954f50d 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -566,6 +566,7 @@ struct dev_pm_info {
boolignore_children:1;
boolearly_init:1;   /* Owned by the PM core */
booldirect_complete:1;  /* Owned by the PM core 
*/
+   unsigned intirq_safe:1; /* PM runtime */
spinlock_t  lock;
 #ifdef CONFIG_PM_SLEEP
struct list_headentry;
@@ -590,7 +591,6 @@ struct dev_pm_info {
unsigned intrun_wake:1;
unsigned intruntime_auto:1;
unsigned intno_callbacks:1;
-   unsigned intirq_safe:1;
unsigned intuse_autosuspend:1;
unsigned inttimer_autosuspends:1;
unsigned intmemalloc_noio:1;
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 367f49b9a1c9..d94a65662a60 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -166,7 +166,10 @@ static inline bool pm_runtime_suspended_if_enabled(struct 
device *dev) { return
 static inline bool pm_runtime_enabled(struct device *dev) { return false; }
 
 static inline void pm_runtime_no_callbacks(struct device *dev) {}
-static inline void pm_runtime_irq_safe(struct device *dev) {}
+static inline void pm_runtime_irq_safe(struct device *dev)
+{
+   dev-power.irq_safe = 1;
+}
 
 static inline bool pm_runtime_callbacks_present(struct device *dev) { return 
false; }
 static inline void pm_runtime_mark_last_busy(struct device *dev) {}
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v10 1/5] PM / Runtime: Allow accessing irq_safe if no PM_RUNTIME

2014-11-06 Thread Ulf Hansson
On 6 November 2014 09:36, Krzysztof Kozlowski k.kozlow...@samsung.com wrote:
 Some drivers (e.g. bus drivers) may want to check if power.irq_safe was
 called by child driver, regardless of CONFIG_PM_RUNTIME.

 An example scenario is amba/bus.c and dma/pl330.c drivers. The runtime
 suspend/resume callbacks in amba bus driver act differently if irq_safe
 was set by child driver (in irq_safe mode bus clock is only disabled).

 The pl330 driver sets irq_safe and assumes that amba bus driver will
 only disable the clock in runtime PM. So in system sleep suspend
 callback the pl330 driver unprepares the clock after calling
 pm_runtime_force_suspend().

 However inconsistency would appear if CONFIG_PM_RUNTIME is not set and
 child drivers do not want the irq_safe runtime PM. In such case amba bus
 driver still has to know whether child driver wanted irq_safe - by
 looking at dev-power.irq_safe field.

 Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com

FWIW: Reviewed-by: Ulf Hansson ulf.hans...@linaro.org

Kind regards
Uffe

 ---
  include/linux/pm.h | 2 +-
  include/linux/pm_runtime.h | 5 -
  2 files changed, 5 insertions(+), 2 deletions(-)

 diff --git a/include/linux/pm.h b/include/linux/pm.h
 index 383fd68aaee1..b05fa954f50d 100644
 --- a/include/linux/pm.h
 +++ b/include/linux/pm.h
 @@ -566,6 +566,7 @@ struct dev_pm_info {
 boolignore_children:1;
 boolearly_init:1;   /* Owned by the PM core */
 booldirect_complete:1;  /* Owned by the PM 
 core */
 +   unsigned intirq_safe:1; /* PM runtime */
 spinlock_t  lock;
  #ifdef CONFIG_PM_SLEEP
 struct list_headentry;
 @@ -590,7 +591,6 @@ struct dev_pm_info {
 unsigned intrun_wake:1;
 unsigned intruntime_auto:1;
 unsigned intno_callbacks:1;
 -   unsigned intirq_safe:1;
 unsigned intuse_autosuspend:1;
 unsigned inttimer_autosuspends:1;
 unsigned intmemalloc_noio:1;
 diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
 index 367f49b9a1c9..d94a65662a60 100644
 --- a/include/linux/pm_runtime.h
 +++ b/include/linux/pm_runtime.h
 @@ -166,7 +166,10 @@ static inline bool 
 pm_runtime_suspended_if_enabled(struct device *dev) { return
  static inline bool pm_runtime_enabled(struct device *dev) { return false; }

  static inline void pm_runtime_no_callbacks(struct device *dev) {}
 -static inline void pm_runtime_irq_safe(struct device *dev) {}
 +static inline void pm_runtime_irq_safe(struct device *dev)
 +{
 +   dev-power.irq_safe = 1;
 +}

  static inline bool pm_runtime_callbacks_present(struct device *dev) { return 
 false; }
  static inline void pm_runtime_mark_last_busy(struct device *dev) {}
 --
 1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/