All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduardo Valentin <eduardo.valentin@ti.com>
To: Nishanth Menon <nm@ti.com>
Cc: Kevin Hilman <khilman@ti.com>, Tony Lindgren <tony@atomide.com>,
	Wenbiao Wang <wwang@ti.com>,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH V2] ARM: OMAP3+: PM: VP: ensure VP is idle before disable
Date: Sat, 19 May 2012 12:52:37 +0300	[thread overview]
Message-ID: <20120519095237.GA26761@besouro> (raw)
In-Reply-To: <1337365122-702-1-git-send-email-nm@ti.com>

Hello,

Minor suggestion.

On Fri, May 18, 2012 at 01:18:41PM -0500, ext Nishanth Menon wrote:
> From: Wenbiao Wang <wwang@ti.com>
> 
> Voltage Processor state machine transition to disable need to
> occur from IDLE state. When we transition OPP in a functioning
> system, the call sequence for an OPP transition is as follows:
> omap_sr_disable
>       -> sr class 3 disable
>            -> vp disable
>            -> sr disable
> forceupdate to voltage/frequency scale depending on which OPP
> we are transitioning to.
> 
> If we hit a critical timing window where SR had commanded VP
> for a voltage transition and VP is in the middle of operating
> on that command, it needs to go through a few states before
> going to update state(where it actually sends the command to
> VC). Initial view of h/w owners is that the state disable of VP
> is expected to be sampled for the next transition.
> 
> Instead, to be on a safer side, we ensure that the valid states
> of the VP state machine is diligently followed by software. This
> can be done by waiting for VP to be in idle  prior to disabling
> VP. Existing prints have been updated to ensure context is
> available on error messages.
> 
> As part of this change, increase timeout for VP idle check to
> improbable 500uSec to be certain that system is indeed unable
> to continue before crashing out with error(worst case expectancy
> remains the same 3-100uSec depending on when we caught VP).
> 


<cut>

>  
>  /* XXX document */
> -#define VP_IDLE_TIMEOUT		200
> +#define VP_IDLE_TIMEOUT		500

I guess it is time to properly document this increasing busy loop delay..
As it is getting closer to ms scale..

>  #define VP_TRANXDONE_TIMEOUT	300
>  
>  /**
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: eduardo.valentin@ti.com (Eduardo Valentin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2] ARM: OMAP3+: PM: VP: ensure VP is idle before disable
Date: Sat, 19 May 2012 12:52:37 +0300	[thread overview]
Message-ID: <20120519095237.GA26761@besouro> (raw)
In-Reply-To: <1337365122-702-1-git-send-email-nm@ti.com>

Hello,

Minor suggestion.

On Fri, May 18, 2012 at 01:18:41PM -0500, ext Nishanth Menon wrote:
> From: Wenbiao Wang <wwang@ti.com>
> 
> Voltage Processor state machine transition to disable need to
> occur from IDLE state. When we transition OPP in a functioning
> system, the call sequence for an OPP transition is as follows:
> omap_sr_disable
>       -> sr class 3 disable
>            -> vp disable
>            -> sr disable
> forceupdate to voltage/frequency scale depending on which OPP
> we are transitioning to.
> 
> If we hit a critical timing window where SR had commanded VP
> for a voltage transition and VP is in the middle of operating
> on that command, it needs to go through a few states before
> going to update state(where it actually sends the command to
> VC). Initial view of h/w owners is that the state disable of VP
> is expected to be sampled for the next transition.
> 
> Instead, to be on a safer side, we ensure that the valid states
> of the VP state machine is diligently followed by software. This
> can be done by waiting for VP to be in idle  prior to disabling
> VP. Existing prints have been updated to ensure context is
> available on error messages.
> 
> As part of this change, increase timeout for VP idle check to
> improbable 500uSec to be certain that system is indeed unable
> to continue before crashing out with error(worst case expectancy
> remains the same 3-100uSec depending on when we caught VP).
> 


<cut>

>  
>  /* XXX document */
> -#define VP_IDLE_TIMEOUT		200
> +#define VP_IDLE_TIMEOUT		500

I guess it is time to properly document this increasing busy loop delay..
As it is getting closer to ms scale..

>  #define VP_TRANXDONE_TIMEOUT	300
>  
>  /**
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2012-05-19  9:52 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <[PATCH] ARM: OMAP3+: PM: VP: ensure VP is idle before disable>
2012-05-18 18:18 ` [PATCH V2] ARM: OMAP3+: PM: VP: ensure VP is idle before disable Nishanth Menon
2012-05-18 18:18   ` Nishanth Menon
2012-05-19  9:52   ` Eduardo Valentin [this message]
2012-05-19  9:52     ` Eduardo Valentin
2012-05-21 13:36     ` Menon, Nishanth
2012-05-21 13:36       ` Menon, Nishanth
2012-05-21 14:51       ` Eduardo Valentin
2012-05-21 14:51         ` Eduardo Valentin
2012-05-21 15:14         ` Menon, Nishanth
2012-05-21 15:14           ` Menon, Nishanth
2012-05-22  7:02           ` Valentin, Eduardo
2012-05-22  7:02             ` Valentin, Eduardo
2012-05-18 19:33 ` [PATCH 0/3] ARM: OMAP3+: VP: collate fixes Nishanth Menon
2012-05-18 19:33   ` Nishanth Menon
2012-05-18 19:33   ` [PATCH 1/3] ARM: OMAP3+: PM: VP: ensure VP is idle before disable Nishanth Menon
2012-05-18 19:33     ` Nishanth Menon
2012-05-31 23:21     ` Kevin Hilman
2012-05-31 23:21       ` Kevin Hilman
2012-05-31 23:28       ` Kevin Hilman
2012-05-31 23:28         ` Kevin Hilman
2012-05-31 23:35       ` Nishanth Menon
2012-05-31 23:35         ` Nishanth Menon
2012-05-18 19:33   ` [PATCH 2/3] ARM: OMAP3+: PM: VP: check to ensure VP is idle before forceupdate Nishanth Menon
2012-05-18 19:33     ` Nishanth Menon
2012-05-31 23:23     ` Kevin Hilman
2012-05-31 23:23       ` Kevin Hilman
2012-05-31 23:37       ` Nishanth Menon
2012-05-31 23:37         ` Nishanth Menon
2012-05-18 19:33   ` [PATCH 3/3] ARM: OMAP3+: PM: VP: check only the VPINIDLE status bit Nishanth Menon
2012-05-18 19:33     ` Nishanth Menon
2012-05-31 23:27     ` Kevin Hilman
2012-05-31 23:27       ` Kevin Hilman
2012-05-31 23:39       ` Nishanth Menon
2012-05-31 23:39         ` Nishanth Menon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120519095237.GA26761@besouro \
    --to=eduardo.valentin@ti.com \
    --cc=khilman@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=tony@atomide.com \
    --cc=wwang@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.