Hi Tero,

On Fri, 2011-01-21 at 12:37 +0200, Tero Kristo wrote:
> On OMAP3 SoCs, if the CORE powerdomain enters off-mode, many other
> parts of the chip will be reset.  If those parts of the chip are busy,
> the reset will disrupt them, causing unpredictable and generally
> undesirable results. This reset is caused by the core domain wakeup
> (COREDOMAINWKUP_RST), and it can occur for example in a case where
> some peripheral domain is in retention or inactive state and core
> enters off. This will result in the peripheral domain being reset.

Found a minor bug here...

> Signed-off-by: Tero Kristo <tero.kri...@nokia.com>
> Cc: Paul Walmsley <p...@pwsan.com>
> Cc: Kevin Hilman <khil...@deeprootsystems.com>
> Cc: Santosh Shilimkar <santosh.shilim...@ti.com>
> Cc: Vishwanath Sripathy <vishwanath...@ti.com>
> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |   40 +++++++++++++++++++++++++++++++++++-
>  1 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
> b/arch/arm/mach-omap2/cpuidle34xx.c
> index f3e043f..d31b858 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -61,7 +61,7 @@ struct omap3_processor_cx {
>  struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
>  struct omap3_processor_cx current_cx_state;
>  struct powerdomain *mpu_pd, *core_pd, *per_pd;
> -struct powerdomain *cam_pd;
> +struct powerdomain *cam_pd, *dss_pd, *iva2_pd, *sgx_pd, *usb_pd;
>  
>  /*
>   * The latencies/thresholds for various C states have
> @@ -235,7 +235,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  {
>       struct cpuidle_state *new_state = next_valid_state(dev, state);
>       u32 core_next_state, per_next_state = 0, per_saved_state = 0;
> -     u32 cam_state;
> +     u32 cam_state, dss_state, iva2_state, sgx_state, usb_state;
>       struct omap3_processor_cx *cx;
>       int ret;
>  
> @@ -256,6 +256,8 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>        *        its own code.
>        */
>  
> +     /* XXX Add CORE-active check here */
> +
>       /*
>        * Prevent idle completely if CAM is active.
>        * CAM does not have wakeup capability in OMAP3.
> @@ -275,6 +277,36 @@ static int omap3_enter_idle_bm(struct cpuidle_device 
> *dev,
>           (core_next_state > PWRDM_POWER_RET))
>               per_next_state = PWRDM_POWER_RET;
>  
> +     /* XXX Add prevent-PER-off check here */
> +
> +     /*
> +      * If we are attempting CORE off, check if any other powerdomains
> +      * are at retention or higher. CORE off causes chipwide reset which
> +      * would reset these domains also.
> +      */
> +     if (core_next_state == PWRDM_POWER_OFF) {
> +             iva2_state = pwrdm_read_pwrst(iva2_pd);
> +             sgx_state = pwrdm_read_pwrst(sgx_pd);
> +             usb_state = pwrdm_read_pwrst(usb_pd);
> +             dss_state = pwrdm_read_pwrst(dss_pd);
> +
> +             if (cam_state > PWRDM_POWER_OFF ||
> +                 dss_state > PWRDM_POWER_OFF ||
> +                 iva2_state > PWRDM_POWER_OFF ||
> +                 per_next_state > PWRDM_POWER_OFF ||
> +                 sgx_state > PWRDM_POWER_OFF ||
> +                 usb_state > PWRDM_POWER_OFF)
> +                     core_next_state = PWRDM_POWER_RET;
> +     }
> +
> +     /* Fallback to new target core/mpu state */
> +     while (cx->core_state < core_next_state) {
> +             state--;

This can potentially pick a state that has its 'valid' flag set to zero,
for example, a state with MPU/CORE set to off-mode, but
enable_off_mode=0.

The fix is to also add a check for cx->valid in the while test.  Will
fold that fix into original patch and queue for 2.6.39.

Kevin

> +             cx = cpuidle_get_statedata(state);
> +     }
> +
> +     new_state = state;
> +
>       /* Are we changing PER target state? */
>       if (per_next_state != per_saved_state)
>               pwrdm_set_next_pwrst(per_pd, per_next_state);
> @@ -489,6 +521,10 @@ int __init omap3_idle_init(void)
>       core_pd = pwrdm_lookup("core_pwrdm");
>       per_pd = pwrdm_lookup("per_pwrdm");
>       cam_pd = pwrdm_lookup("cam_pwrdm");
> +     dss_pd = pwrdm_lookup("dss_pwrdm");
> +     iva2_pd = pwrdm_lookup("iva2_pwrdm");
> +     sgx_pd = pwrdm_lookup("sgx_pwrdm");
> +     usb_pd = pwrdm_lookup("usbhost_pwrdm");
>  
>       omap_init_power_states();
>       cpuidle_register_driver(&omap3_idle_driver);


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

Reply via email to