Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-30 Thread Kevin Hilman
Menon, Nishanth n...@ti.com writes:

 On Thu, May 17, 2012 at 3:52 AM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 12:34 PM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 4:12 AM, Kevin Hilman khil...@ti.com wrote:
 Tero Kristo t-kri...@ti.com writes:

 [...]
 - Rather than hooking into omap4_enter_lowpower(), should use
  the cluster PM enter/exit notifier chain.

 This is again specific to device OFF only and not related to CPU
 cluster state as such. So I don't think notifiers should be used here.

 O.w even when we attempt just MPU OSWR C-state, all these functions will
 get called in notifier chain.

 Just a thought, we can have a separate notifier chain for device OFF. It can
 allow use to get rid of 'enable_off_mode kind of flags and can be
 used by many drivers too.

 Like the overall idea, but one minor dumb concern might be sequencing
 of notifiers
  - OFF entry and restore needs things to be executed in a specific sequence.
 How do we plan to ensure the sequence is maintained in a notifier call
 chain? one
 possible option might be a priority based scheme?

Or just combine the events that need a specific sequence into single
notifier callback function.

Kevin

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


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-30 Thread Menon, Nishanth
On Wed, May 30, 2012 at 12:59 PM, Kevin Hilman khil...@ti.com wrote:
 Menon, Nishanth n...@ti.com writes:

 On Thu, May 17, 2012 at 3:52 AM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 12:34 PM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 4:12 AM, Kevin Hilman khil...@ti.com wrote:
 Tero Kristo t-kri...@ti.com writes:

 [...]
 - Rather than hooking into omap4_enter_lowpower(), should use
  the cluster PM enter/exit notifier chain.

 This is again specific to device OFF only and not related to CPU
 cluster state as such. So I don't think notifiers should be used here.

 O.w even when we attempt just MPU OSWR C-state, all these functions will
 get called in notifier chain.

 Just a thought, we can have a separate notifier chain for device OFF. It can
 allow use to get rid of 'enable_off_mode kind of flags and can be
 used by many drivers too.

 Like the overall idea, but one minor dumb concern might be sequencing
 of notifiers
  - OFF entry and restore needs things to be executed in a specific sequence.
 How do we plan to ensure the sequence is maintained in a notifier call
 chain? one
 possible option might be a priority based scheme?

 Or just combine the events that need a specific sequence into single
 notifier callback function.
There is other issues in case of failure cases - abort of OFF
sequence due to pending interrupt
detected as part of a notifier - error handling needs to be sane in
proper sequence.
I understand and appreciate the intent of replacing the single mega
enter_sleep with a chain of notifiers
but any such option will need to be scalable enough to handle weird
erratum handling (HSI CAWAKE as an example)
which potentially break the logic flow and be either be equal or
better than what we have today interms of
error handling. since these notifiers will be triggered for
CPUIDLE(performance sensitive) and suspend, the intricacies
might be better understood by seeing how this proposed notifiers look like.

Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-30 Thread Kevin Hilman
Menon, Nishanth n...@ti.com writes:

 On Wed, May 30, 2012 at 12:59 PM, Kevin Hilman khil...@ti.com wrote:
 Menon, Nishanth n...@ti.com writes:

 On Thu, May 17, 2012 at 3:52 AM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 12:34 PM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 4:12 AM, Kevin Hilman khil...@ti.com wrote:
 Tero Kristo t-kri...@ti.com writes:

 [...]
 - Rather than hooking into omap4_enter_lowpower(), should use
  the cluster PM enter/exit notifier chain.

 This is again specific to device OFF only and not related to CPU
 cluster state as such. So I don't think notifiers should be used here.

 O.w even when we attempt just MPU OSWR C-state, all these functions will
 get called in notifier chain.

 Just a thought, we can have a separate notifier chain for device OFF. It 
 can
 allow use to get rid of 'enable_off_mode kind of flags and can be
 used by many drivers too.

 Like the overall idea, but one minor dumb concern might be sequencing
 of notifiers
  - OFF entry and restore needs things to be executed in a specific sequence.
 How do we plan to ensure the sequence is maintained in a notifier call
 chain? one
 possible option might be a priority based scheme?

 Or just combine the events that need a specific sequence into single
 notifier callback function.
 There is other issues in case of failure cases - abort of OFF
 sequence due to pending interrupt
 detected as part of a notifier - error handling needs to be sane in
 proper sequence.
 I understand and appreciate the intent of replacing the single mega
 enter_sleep with a chain of notifiers
 but any such option will need to be scalable enough to handle weird
 erratum handling (HSI CAWAKE as an example)
 which potentially break the logic flow and be either be equal or
 better than what we have today interms of
 error handling. since these notifiers will be triggered for
 CPUIDLE(performance sensitive) and suspend, the intricacies
 might be better understood by seeing how this proposed notifiers look like.

Makes sense.  Thanks for clarifying.

What $SUBJECT series proposed was indeed a mega function, but one that
was just a list of function calls with no error checking or recovery,
and no documentation/description about dependencies/sequencing etc. etc.
Based on the patches at hadn (which is all reviewers have to go on),
notifiers seemed to be a good fit.

If there are good reasons that all of the device-off events need to be
coordinated/synchronized/sequenced (and it sounds like there are, thanks
for pointing them out), I am not opposed to that approach.  It simply
needs to be well described in the changlog.

Thanks,

Kevin





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


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-30 Thread Shilimkar, Santosh
On Thu, May 31, 2012 at 3:39 AM, Kevin Hilman khil...@ti.com wrote:
 Menon, Nishanth n...@ti.com writes:

 On Wed, May 30, 2012 at 12:59 PM, Kevin Hilman khil...@ti.com wrote:
 Menon, Nishanth n...@ti.com writes:

 On Thu, May 17, 2012 at 3:52 AM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 12:34 PM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 4:12 AM, Kevin Hilman khil...@ti.com wrote:
 Tero Kristo t-kri...@ti.com writes:

 [...]
 - Rather than hooking into omap4_enter_lowpower(), should use
  the cluster PM enter/exit notifier chain.

 This is again specific to device OFF only and not related to CPU
 cluster state as such. So I don't think notifiers should be used here.

 O.w even when we attempt just MPU OSWR C-state, all these functions will
 get called in notifier chain.

 Just a thought, we can have a separate notifier chain for device OFF. It 
 can
 allow use to get rid of 'enable_off_mode kind of flags and can be
 used by many drivers too.

 Like the overall idea, but one minor dumb concern might be sequencing
 of notifiers
  - OFF entry and restore needs things to be executed in a specific 
 sequence.
 How do we plan to ensure the sequence is maintained in a notifier call
 chain? one
 possible option might be a priority based scheme?

 Or just combine the events that need a specific sequence into single
 notifier callback function.
 There is other issues in case of failure cases - abort of OFF
 sequence due to pending interrupt
 detected as part of a notifier - error handling needs to be sane in
 proper sequence.
 I understand and appreciate the intent of replacing the single mega
 enter_sleep with a chain of notifiers
 but any such option will need to be scalable enough to handle weird
 erratum handling (HSI CAWAKE as an example)
 which potentially break the logic flow and be either be equal or
 better than what we have today interms of
 error handling. since these notifiers will be triggered for
 CPUIDLE(performance sensitive) and suspend, the intricacies
 might be better understood by seeing how this proposed notifiers look like.

 Makes sense.  Thanks for clarifying.

 What $SUBJECT series proposed was indeed a mega function, but one that
 was just a list of function calls with no error checking or recovery,
 and no documentation/description about dependencies/sequencing etc. etc.
 Based on the patches at hadn (which is all reviewers have to go on),
 notifiers seemed to be a good fit.

 If there are good reasons that all of the device-off events need to be
 coordinated/synchronized/sequenced (and it sounds like there are, thanks
 for pointing them out), I am not opposed to that approach.  It simply
 needs to be well described in the changlog.

There are sequence dependencies but lot of code can be extracted and put
into smaller blocks that is independent.

I agree for the error handling part notifier chain could be an issue.

Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-29 Thread Menon, Nishanth
On Thu, May 17, 2012 at 3:52 AM, Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 12:34 PM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 4:12 AM, Kevin Hilman khil...@ti.com wrote:
 Tero Kristo t-kri...@ti.com writes:

[...]
 - Rather than hooking into omap4_enter_lowpower(), should use
  the cluster PM enter/exit notifier chain.

 This is again specific to device OFF only and not related to CPU
 cluster state as such. So I don't think notifiers should be used here.

 O.w even when we attempt just MPU OSWR C-state, all these functions will
 get called in notifier chain.

 Just a thought, we can have a separate notifier chain for device OFF. It can
 allow use to get rid of 'enable_off_mode kind of flags and can be
 used by many drivers too.

Like the overall idea, but one minor dumb concern might be sequencing
of notifiers
 - OFF entry and restore needs things to be executed in a specific sequence.
How do we plan to ensure the sequence is maintained in a notifier call
chain? one
possible option might be a priority based scheme?

Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-21 Thread Tero Kristo
On Wed, 2012-05-16 at 15:42 -0700, Kevin Hilman wrote:
 Tero Kristo t-kri...@ti.com writes:
 
  From: Rajendra Nayak rna...@ti.com
 
  SAR/ROM code restores only CORE DPLL to its original state
  post wakeup from OFF mode.
  The rest of the DPLL's in OMAP4 platform (MPU/IVA/ABE/USB/PER)
  are saved and restored here during an OFF transition.
 
  [n...@ti.com: minor cleanups]
  Signed-off-by: Nishanth Menon n...@ti.com
  Signed-off-by: Rajendra Nayak rna...@ti.com
  Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
  Signed-off-by: Tero Kristo t-kri...@ti.com
 
 Some general comments:
 
 - the register dump/print doesn't belong here.  If needed, should be a
   debug feature if needed.

Okay, I'll just drop it out.

 
 - Rather than hooking into omap4_enter_lowpower(), should use
   the cluster PM enter/exit notifier chain.

Yes can do that.

-Tero

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


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-21 Thread Tero Kristo
On Thu, 2012-05-17 at 09:37 -0700, Kevin Hilman wrote:
 Shilimkar, Santosh santosh.shilim...@ti.com writes:
 
  On Thu, May 17, 2012 at 12:34 PM, Shilimkar, Santosh
  santosh.shilim...@ti.com wrote:
  On Thu, May 17, 2012 at 4:12 AM, Kevin Hilman khil...@ti.com wrote:
  Tero Kristo t-kri...@ti.com writes:
 
  From: Rajendra Nayak rna...@ti.com
 
  SAR/ROM code restores only CORE DPLL to its original state
  post wakeup from OFF mode.
  The rest of the DPLL's in OMAP4 platform (MPU/IVA/ABE/USB/PER)
  are saved and restored here during an OFF transition.
 
  [n...@ti.com: minor cleanups]
  Signed-off-by: Nishanth Menon n...@ti.com
  Signed-off-by: Rajendra Nayak rna...@ti.com
  Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
  Signed-off-by: Tero Kristo t-kri...@ti.com
 
  Some general comments:
 
  - the register dump/print doesn't belong here.  If needed, should be a
   debug feature if needed.
 
  - Rather than hooking into omap4_enter_lowpower(), should use
   the cluster PM enter/exit notifier chain.
 
  This is again specific to device OFF only and not related to CPU
  cluster state as such. So I don't think notifiers should be used here.
 
  O.w even when we attempt just MPU OSWR C-state, all these functions will
  get called in notifier chain.
 
  Just a thought, we can have a separate notifier chain for device OFF. It can
  allow use to get rid of 'enable_off_mode kind of flags and can be
  used by many drivers too.
 
 Yes, I like this idea.
 
 It allows a much cleaner collection of all the activities needed for
 device off.

I can just read the device next state off in the notifier func.

-Tero


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


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-17 Thread Shilimkar, Santosh
On Thu, May 17, 2012 at 4:12 AM, Kevin Hilman khil...@ti.com wrote:
 Tero Kristo t-kri...@ti.com writes:

 From: Rajendra Nayak rna...@ti.com

 SAR/ROM code restores only CORE DPLL to its original state
 post wakeup from OFF mode.
 The rest of the DPLL's in OMAP4 platform (MPU/IVA/ABE/USB/PER)
 are saved and restored here during an OFF transition.

 [n...@ti.com: minor cleanups]
 Signed-off-by: Nishanth Menon n...@ti.com
 Signed-off-by: Rajendra Nayak rna...@ti.com
 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 Signed-off-by: Tero Kristo t-kri...@ti.com

 Some general comments:

 - the register dump/print doesn't belong here.  If needed, should be a
  debug feature if needed.

 - Rather than hooking into omap4_enter_lowpower(), should use
  the cluster PM enter/exit notifier chain.

This is again specific to device OFF only and not related to CPU
cluster state as such. So I don't think notifiers should be used here.

O.w even when we attempt just MPU OSWR C-state, all these functions will
get called in notifier chain.

Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-17 Thread Shilimkar, Santosh
On Thu, May 17, 2012 at 12:34 PM, Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 4:12 AM, Kevin Hilman khil...@ti.com wrote:
 Tero Kristo t-kri...@ti.com writes:

 From: Rajendra Nayak rna...@ti.com

 SAR/ROM code restores only CORE DPLL to its original state
 post wakeup from OFF mode.
 The rest of the DPLL's in OMAP4 platform (MPU/IVA/ABE/USB/PER)
 are saved and restored here during an OFF transition.

 [n...@ti.com: minor cleanups]
 Signed-off-by: Nishanth Menon n...@ti.com
 Signed-off-by: Rajendra Nayak rna...@ti.com
 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 Signed-off-by: Tero Kristo t-kri...@ti.com

 Some general comments:

 - the register dump/print doesn't belong here.  If needed, should be a
  debug feature if needed.

 - Rather than hooking into omap4_enter_lowpower(), should use
  the cluster PM enter/exit notifier chain.

 This is again specific to device OFF only and not related to CPU
 cluster state as such. So I don't think notifiers should be used here.

 O.w even when we attempt just MPU OSWR C-state, all these functions will
 get called in notifier chain.

Just a thought, we can have a separate notifier chain for device OFF. It can
allow use to get rid of 'enable_off_mode kind of flags and can be
used by many drivers too.

Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-17 Thread Kevin Hilman
Shilimkar, Santosh santosh.shilim...@ti.com writes:

 On Thu, May 17, 2012 at 12:34 PM, Shilimkar, Santosh
 santosh.shilim...@ti.com wrote:
 On Thu, May 17, 2012 at 4:12 AM, Kevin Hilman khil...@ti.com wrote:
 Tero Kristo t-kri...@ti.com writes:

 From: Rajendra Nayak rna...@ti.com

 SAR/ROM code restores only CORE DPLL to its original state
 post wakeup from OFF mode.
 The rest of the DPLL's in OMAP4 platform (MPU/IVA/ABE/USB/PER)
 are saved and restored here during an OFF transition.

 [n...@ti.com: minor cleanups]
 Signed-off-by: Nishanth Menon n...@ti.com
 Signed-off-by: Rajendra Nayak rna...@ti.com
 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 Signed-off-by: Tero Kristo t-kri...@ti.com

 Some general comments:

 - the register dump/print doesn't belong here.  If needed, should be a
  debug feature if needed.

 - Rather than hooking into omap4_enter_lowpower(), should use
  the cluster PM enter/exit notifier chain.

 This is again specific to device OFF only and not related to CPU
 cluster state as such. So I don't think notifiers should be used here.

 O.w even when we attempt just MPU OSWR C-state, all these functions will
 get called in notifier chain.

 Just a thought, we can have a separate notifier chain for device OFF. It can
 allow use to get rid of 'enable_off_mode kind of flags and can be
 used by many drivers too.

Yes, I like this idea.

It allows a much cleaner collection of all the activities needed for
device off.

Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 04/19] ARM: OMAP4: PM: save/restore all DPLL settings in OFF mode

2012-05-16 Thread Kevin Hilman
Tero Kristo t-kri...@ti.com writes:

 From: Rajendra Nayak rna...@ti.com

 SAR/ROM code restores only CORE DPLL to its original state
 post wakeup from OFF mode.
 The rest of the DPLL's in OMAP4 platform (MPU/IVA/ABE/USB/PER)
 are saved and restored here during an OFF transition.

 [n...@ti.com: minor cleanups]
 Signed-off-by: Nishanth Menon n...@ti.com
 Signed-off-by: Rajendra Nayak rna...@ti.com
 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 Signed-off-by: Tero Kristo t-kri...@ti.com

Some general comments:

- the register dump/print doesn't belong here.  If needed, should be a
  debug feature if needed.

- Rather than hooking into omap4_enter_lowpower(), should use
  the cluster PM enter/exit notifier chain.

Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html