All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH V2 3/3] ARM: tegra114: cpuidle: add powered-down state
Date: Tue, 04 Jun 2013 23:40:51 +0200	[thread overview]
Message-ID: <51AE5EE3.3010607@linaro.org> (raw)
In-Reply-To: <1370342880-422-4-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

On 06/04/2013 12:48 PM, Joseph Lo wrote:
> This supports CPU core power down on each CPU when CPU idle. When CPU go
> into this state, it saves it's context and needs a proper configuration
> in flow controller to power gate the CPU when CPU runs into WFI
> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> wake up event in flow controller.
> 
> Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

I would like to understand why there is a WARN with the
CPUIDLE_FLAG_TIMER_STOP flag set before queuing this patch and ensure it
is not the tree hiding the forest.

Thanks
  -- Daniel

> ---
> V2:
> * remove some redundant code of memory barrier
> * remove the function declaration by rearranging the coding sequence 
> ---
>  arch/arm/mach-tegra/cpuidle-tegra114.c | 51 +++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> index 1d1c602..e0b8730 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
> @@ -17,15 +17,64 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/clockchips.h>
>  
>  #include <asm/cpuidle.h>
> +#include <asm/suspend.h>
> +#include <asm/smp_plat.h>
> +
> +#include "pm.h"
> +#include "sleep.h"
> +
> +#ifdef CONFIG_PM_SLEEP
> +#define TEGRA114_MAX_STATES 2
> +#else
> +#define TEGRA114_MAX_STATES 1
> +#endif
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> +				    struct cpuidle_driver *drv,
> +				    int index)
> +{
> +	local_fiq_disable();
> +
> +	tegra_set_cpu_in_lp2();
> +	cpu_pm_enter();
> +
> +	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> +
> +	cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> +
> +	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> +
> +	cpu_pm_exit();
> +	tegra_clear_cpu_in_lp2();
> +
> +	local_fiq_enable();
> +
> +	return index;
> +}
> +#endif
>  
>  static struct cpuidle_driver tegra_idle_driver = {
>  	.name = "tegra_idle",
>  	.owner = THIS_MODULE,
> -	.state_count = 1,
> +	.state_count = TEGRA114_MAX_STATES,
>  	.states = {
>  		[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> +#ifdef CONFIG_PM_SLEEP
> +		[1] = {
> +			.enter			= tegra114_idle_power_down,
> +			.exit_latency		= 500,
> +			.target_residency	= 1000,
> +			.power_usage		= 0,
> +			.flags			= CPUIDLE_FLAG_TIME_VALID,
> +			.name			= "powered-down",
> +			.desc			= "CPU power gated",
> +		},
> +#endif
>  	},
>  };
>  
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

WARNING: multiple messages have this Message-ID (diff)
From: daniel.lezcano@linaro.org (Daniel Lezcano)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2 3/3] ARM: tegra114: cpuidle: add powered-down state
Date: Tue, 04 Jun 2013 23:40:51 +0200	[thread overview]
Message-ID: <51AE5EE3.3010607@linaro.org> (raw)
In-Reply-To: <1370342880-422-4-git-send-email-josephl@nvidia.com>

On 06/04/2013 12:48 PM, Joseph Lo wrote:
> This supports CPU core power down on each CPU when CPU idle. When CPU go
> into this state, it saves it's context and needs a proper configuration
> in flow controller to power gate the CPU when CPU runs into WFI
> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> wake up event in flow controller.
> 
> Signed-off-by: Joseph Lo <josephl@nvidia.com>

I would like to understand why there is a WARN with the
CPUIDLE_FLAG_TIMER_STOP flag set before queuing this patch and ensure it
is not the tree hiding the forest.

Thanks
  -- Daniel

> ---
> V2:
> * remove some redundant code of memory barrier
> * remove the function declaration by rearranging the coding sequence 
> ---
>  arch/arm/mach-tegra/cpuidle-tegra114.c | 51 +++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> index 1d1c602..e0b8730 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
> @@ -17,15 +17,64 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/clockchips.h>
>  
>  #include <asm/cpuidle.h>
> +#include <asm/suspend.h>
> +#include <asm/smp_plat.h>
> +
> +#include "pm.h"
> +#include "sleep.h"
> +
> +#ifdef CONFIG_PM_SLEEP
> +#define TEGRA114_MAX_STATES 2
> +#else
> +#define TEGRA114_MAX_STATES 1
> +#endif
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra114_idle_power_down(struct cpuidle_device *dev,
> +				    struct cpuidle_driver *drv,
> +				    int index)
> +{
> +	local_fiq_disable();
> +
> +	tegra_set_cpu_in_lp2();
> +	cpu_pm_enter();
> +
> +	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> +
> +	cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> +
> +	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> +
> +	cpu_pm_exit();
> +	tegra_clear_cpu_in_lp2();
> +
> +	local_fiq_enable();
> +
> +	return index;
> +}
> +#endif
>  
>  static struct cpuidle_driver tegra_idle_driver = {
>  	.name = "tegra_idle",
>  	.owner = THIS_MODULE,
> -	.state_count = 1,
> +	.state_count = TEGRA114_MAX_STATES,
>  	.states = {
>  		[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> +#ifdef CONFIG_PM_SLEEP
> +		[1] = {
> +			.enter			= tegra114_idle_power_down,
> +			.exit_latency		= 500,
> +			.target_residency	= 1000,
> +			.power_usage		= 0,
> +			.flags			= CPUIDLE_FLAG_TIME_VALID,
> +			.name			= "powered-down",
> +			.desc			= "CPU power gated",
> +		},
> +#endif
>  	},
>  };
>  
> 


-- 
 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

  parent reply	other threads:[~2013-06-04 21:40 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-04 10:47 [PATCH V2 0/3] ARM: tegra114: cpuidle: add power down state Joseph Lo
2013-06-04 10:47 ` Joseph Lo
     [not found] ` <1370342880-422-1-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-06-04 10:47   ` [PATCH V2 1/3] ARM: tegra114: Reprogram GIC CPU interface to bypass IRQ on CPU PM entry Joseph Lo
2013-06-04 10:47     ` Joseph Lo
2013-06-04 10:47   ` [PATCH V2 2/3] ARM: tegra114: add low level support for CPU idle powered-down mode Joseph Lo
2013-06-04 10:47     ` Joseph Lo
2013-06-04 10:48   ` [PATCH V2 3/3] ARM: tegra114: cpuidle: add powered-down state Joseph Lo
2013-06-04 10:48     ` Joseph Lo
     [not found]     ` <1370342880-422-4-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-06-04 21:40       ` Daniel Lezcano [this message]
2013-06-04 21:40         ` Daniel Lezcano
     [not found]         ` <51AE5EE3.3010607-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2013-06-12 17:41           ` Stephen Warren
2013-06-12 17:41             ` Stephen Warren
     [not found]             ` <51B8B2C8.1090006-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-06-13  2:13               ` Joseph Lo
2013-06-13  2:13                 ` Joseph Lo
     [not found]                 ` <1371089619.1681.21.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
2013-06-13  9:30                   ` Daniel Lezcano
2013-06-13  9:30                     ` Daniel Lezcano
     [not found]                     ` <51B99151.5010107-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2013-06-13 14:32                       ` Joseph Lo
2013-06-13 14:32                         ` Joseph Lo
     [not found]                         ` <1371133960.1631.13.camel-yx3yKKdKkHfc7b1ADBJPm0n48jw8i0AO@public.gmane.org>
2013-06-13 15:02                           ` Daniel Lezcano
2013-06-13 15:02                             ` Daniel Lezcano

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=51AE5EE3.3010607@linaro.org \
    --to=daniel.lezcano-qsej5fyqhm4dnm+yrofe0a@public.gmane.org \
    --cc=josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org \
    /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.