* [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised
@ 2014-10-30 3:55 Amit Daniel Kachhap
2014-10-30 3:55 ` [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index Amit Daniel Kachhap
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Amit Daniel Kachhap @ 2014-10-30 3:55 UTC (permalink / raw)
To: linux-arm-kernel, linux-pm
Cc: Catalin Marinas, Mark Rutland, Lorenzo Pieralisi, Ashwin Chaugule,
Vladimir Murzin
Without this cpu_suspend may cause crash dump when psci cpuidle
is not initialised and cpu_suspend is called.
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
arch/arm64/kernel/psci.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 866c1c8..2178d6e 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -523,9 +523,11 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
struct psci_power_state *state = __get_cpu_var(psci_power_state);
/*
* idle state index 0 corresponds to wfi, should never be called
- * from the cpu_suspend operations
+ * from the cpu_suspend operations.
+ * Also psci_power_state variable should have been populated by
+ * above init idle routine.
*/
- if (WARN_ON_ONCE(!index))
+ if (WARN_ON_ONCE(!index || !state))
return -EINVAL;
if (state->type == PSCI_POWER_STATE_TYPE_STANDBY)
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index
2014-10-30 3:55 [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised Amit Daniel Kachhap
@ 2014-10-30 3:55 ` Amit Daniel Kachhap
2014-10-30 10:19 ` Lorenzo Pieralisi
2014-10-30 3:55 ` [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state Amit Daniel Kachhap
2014-10-30 10:29 ` [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised Lorenzo Pieralisi
2 siblings, 1 reply; 11+ messages in thread
From: Amit Daniel Kachhap @ 2014-10-30 3:55 UTC (permalink / raw)
To: linux-arm-kernel, linux-pm
Cc: Catalin Marinas, Mark Rutland, Lorenzo Pieralisi, Ashwin Chaugule,
Vladimir Murzin
This fix rectifies the psci cpu_suspend to check the C-state type
corresponding to the requested index.
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
arch/arm64/kernel/psci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 2178d6e..4ebc146 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -530,7 +530,7 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
if (WARN_ON_ONCE(!index || !state))
return -EINVAL;
- if (state->type == PSCI_POWER_STATE_TYPE_STANDBY)
+ if (state[index - 1].type == PSCI_POWER_STATE_TYPE_STANDBY)
ret = psci_ops.cpu_suspend(state[index - 1], 0);
else
ret = __cpu_suspend(index, psci_suspend_finisher);
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.
2014-10-30 3:55 [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised Amit Daniel Kachhap
2014-10-30 3:55 ` [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index Amit Daniel Kachhap
@ 2014-10-30 3:55 ` Amit Daniel Kachhap
2014-10-30 10:46 ` Lorenzo Pieralisi
2014-10-30 10:29 ` [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised Lorenzo Pieralisi
2 siblings, 1 reply; 11+ messages in thread
From: Amit Daniel Kachhap @ 2014-10-30 3:55 UTC (permalink / raw)
To: linux-arm-kernel, linux-pm
Cc: Catalin Marinas, Mark Rutland, Lorenzo Pieralisi, Ashwin Chaugule,
Vladimir Murzin
This feature will be useful for system suspend/resume which may
intend to invoke cpu_suspend with deepest possible C state level.
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
arch/arm64/include/asm/psci.h | 2 ++
arch/arm64/kernel/psci.c | 8 ++++++++
2 files changed, 10 insertions(+)
diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h
index e5312ea..487e52b 100644
--- a/arch/arm64/include/asm/psci.h
+++ b/arch/arm64/include/asm/psci.h
@@ -14,6 +14,8 @@
#ifndef __ASM_PSCI_H
#define __ASM_PSCI_H
+#define CPU_PSCI_IDLE_INDEX_MAX 0xff
+
int psci_init(void);
#endif /* __ASM_PSCI_H */
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 4ebc146..a761513 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -70,6 +70,7 @@ enum psci_function {
static DEFINE_PER_CPU_READ_MOSTLY(struct psci_power_state *, psci_power_state);
static u32 psci_function_id[PSCI_FN_MAX];
+static u32 cpu_psci_idle_state_count;
static int psci_to_linux_errno(int errno)
{
@@ -239,6 +240,8 @@ static int __maybe_unused cpu_psci_cpu_init_idle(struct device_node *cpu_node,
if (!count)
return -ENODEV;
+ cpu_psci_idle_state_count = count;
+
psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
if (!psci_states)
return -ENOMEM;
@@ -521,6 +524,11 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
{
int ret;
struct psci_power_state *state = __get_cpu_var(psci_power_state);
+
+ /* Check and select the deepest idle state */
+ if (index == CPU_PSCI_IDLE_INDEX_MAX)
+ index = cpu_psci_idle_state_count;
+
/*
* idle state index 0 corresponds to wfi, should never be called
* from the cpu_suspend operations.
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index
2014-10-30 3:55 ` [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index Amit Daniel Kachhap
@ 2014-10-30 10:19 ` Lorenzo Pieralisi
2014-10-30 11:31 ` amit daniel kachhap
0 siblings, 1 reply; 11+ messages in thread
From: Lorenzo Pieralisi @ 2014-10-30 10:19 UTC (permalink / raw)
To: Amit Daniel Kachhap
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Catalin Marinas, Mark Rutland, Ashwin Chaugule, Vladimir Murzin,
will.deacon
Hi Amit,
On Thu, Oct 30, 2014 at 03:55:37AM +0000, Amit Daniel Kachhap wrote:
> This fix rectifies the psci cpu_suspend to check the C-state type
> corresponding to the requested index.
Can you reword the commit log please ?
"This fix rectifies the psci cpu_suspend implementation to check the
PSCI power state parameter type field associated with the requested idle
state index."
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
> arch/arm64/kernel/psci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index 2178d6e..4ebc146 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -530,7 +530,7 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
> if (WARN_ON_ONCE(!index || !state))
> return -EINVAL;
>
> - if (state->type == PSCI_POWER_STATE_TYPE_STANDBY)
> + if (state[index - 1].type == PSCI_POWER_STATE_TYPE_STANDBY)
> ret = psci_ops.cpu_suspend(state[index - 1], 0);
> else
> ret = __cpu_suspend(index, psci_suspend_finisher);
> --
> 1.9.1
Can you resend it as a single patch (ie this patch does not apply if
patch 1 is missing, and I am not sure we need patch 1) and send it to
Catalin and Will to merge it asap ?
Please add my:
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised
2014-10-30 3:55 [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised Amit Daniel Kachhap
2014-10-30 3:55 ` [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index Amit Daniel Kachhap
2014-10-30 3:55 ` [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state Amit Daniel Kachhap
@ 2014-10-30 10:29 ` Lorenzo Pieralisi
2014-10-30 11:35 ` Amit Kachhap
2 siblings, 1 reply; 11+ messages in thread
From: Lorenzo Pieralisi @ 2014-10-30 10:29 UTC (permalink / raw)
To: Amit Daniel Kachhap
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Catalin Marinas, Mark Rutland, Ashwin Chaugule, Vladimir Murzin
On Thu, Oct 30, 2014 at 03:55:36AM +0000, Amit Daniel Kachhap wrote:
> Without this cpu_suspend may cause crash dump when psci cpuidle
> is not initialised and cpu_suspend is called.
>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
> arch/arm64/kernel/psci.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index 866c1c8..2178d6e 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -523,9 +523,11 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
> struct psci_power_state *state = __get_cpu_var(psci_power_state);
> /*
> * idle state index 0 corresponds to wfi, should never be called
> - * from the cpu_suspend operations
> + * from the cpu_suspend operations.
> + * Also psci_power_state variable should have been populated by
> + * above init idle routine.
> */
> - if (WARN_ON_ONCE(!index))
> + if (WARN_ON_ONCE(!index || !state))
> return -EINVAL;
I thought about this when developing the code, at the moment this can't
happen so I did not add any check for that. I would wait for the dust
to settle on the API usage (and if we need it for something different
than idle) before adding checks that might turn out useless, so we will
keep this patch on the back-burner for now.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.
2014-10-30 3:55 ` [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state Amit Daniel Kachhap
@ 2014-10-30 10:46 ` Lorenzo Pieralisi
2014-10-30 11:22 ` amit daniel kachhap
2014-12-01 8:08 ` amit daniel kachhap
0 siblings, 2 replies; 11+ messages in thread
From: Lorenzo Pieralisi @ 2014-10-30 10:46 UTC (permalink / raw)
To: Amit Daniel Kachhap
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Catalin Marinas, Mark Rutland, Ashwin Chaugule, Vladimir Murzin,
charles.garcia-tobin
On Thu, Oct 30, 2014 at 03:55:38AM +0000, Amit Daniel Kachhap wrote:
> This feature will be useful for system suspend/resume which may
> intend to invoke cpu_suspend with deepest possible C state level.
We are working on defining system states in the PSCI specification,
but it is not set in stone yet (Cc'ed Charles).
I guess you are posting this code because you have already an
implementation that supports system suspend/resume, so I am
taking an immediate action to come up with an API suitable
for that asap.
On a side note I do not necessarily like this magic idle index
logic, I prefer adding an API for that, that translates the call
into whatever is required by the back-end (ie PSCI) implementation.
Thanks,
Lorenzo
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
> arch/arm64/include/asm/psci.h | 2 ++
> arch/arm64/kernel/psci.c | 8 ++++++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h
> index e5312ea..487e52b 100644
> --- a/arch/arm64/include/asm/psci.h
> +++ b/arch/arm64/include/asm/psci.h
> @@ -14,6 +14,8 @@
> #ifndef __ASM_PSCI_H
> #define __ASM_PSCI_H
>
> +#define CPU_PSCI_IDLE_INDEX_MAX 0xff
> +
> int psci_init(void);
>
> #endif /* __ASM_PSCI_H */
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index 4ebc146..a761513 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -70,6 +70,7 @@ enum psci_function {
> static DEFINE_PER_CPU_READ_MOSTLY(struct psci_power_state *, psci_power_state);
>
> static u32 psci_function_id[PSCI_FN_MAX];
> +static u32 cpu_psci_idle_state_count;
>
> static int psci_to_linux_errno(int errno)
> {
> @@ -239,6 +240,8 @@ static int __maybe_unused cpu_psci_cpu_init_idle(struct device_node *cpu_node,
> if (!count)
> return -ENODEV;
>
> + cpu_psci_idle_state_count = count;
> +
> psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
> if (!psci_states)
> return -ENOMEM;
> @@ -521,6 +524,11 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
> {
> int ret;
> struct psci_power_state *state = __get_cpu_var(psci_power_state);
> +
> + /* Check and select the deepest idle state */
> + if (index == CPU_PSCI_IDLE_INDEX_MAX)
> + index = cpu_psci_idle_state_count;
> +
> /*
> * idle state index 0 corresponds to wfi, should never be called
> * from the cpu_suspend operations.
> --
> 1.9.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.
2014-10-30 10:46 ` Lorenzo Pieralisi
@ 2014-10-30 11:22 ` amit daniel kachhap
2014-12-01 8:08 ` amit daniel kachhap
1 sibling, 0 replies; 11+ messages in thread
From: amit daniel kachhap @ 2014-10-30 11:22 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Catalin Marinas, Mark Rutland, Ashwin Chaugule, Vladimir Murzin,
Charles Garcia-Tobin
On Thu, Oct 30, 2014 at 4:16 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Thu, Oct 30, 2014 at 03:55:38AM +0000, Amit Daniel Kachhap wrote:
>> This feature will be useful for system suspend/resume which may
>> intend to invoke cpu_suspend with deepest possible C state level.
>
> We are working on defining system states in the PSCI specification,
> but it is not set in stone yet (Cc'ed Charles).
>
> I guess you are posting this code because you have already an
> implementation that supports system suspend/resume, so I am
> taking an immediate action to come up with an API suitable
> for that asap.
Yes that would be useful. Thanks
>
> On a side note I do not necessarily like this magic idle index
> logic, I prefer adding an API for that, that translates the call
> into whatever is required by the back-end (ie PSCI) implementation.
yeah this is a kind of workaround and API will be better.
>
> Thanks,
> Lorenzo
>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>> arch/arm64/include/asm/psci.h | 2 ++
>> arch/arm64/kernel/psci.c | 8 ++++++++
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h
>> index e5312ea..487e52b 100644
>> --- a/arch/arm64/include/asm/psci.h
>> +++ b/arch/arm64/include/asm/psci.h
>> @@ -14,6 +14,8 @@
>> #ifndef __ASM_PSCI_H
>> #define __ASM_PSCI_H
>>
>> +#define CPU_PSCI_IDLE_INDEX_MAX 0xff
>> +
>> int psci_init(void);
>>
>> #endif /* __ASM_PSCI_H */
>> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
>> index 4ebc146..a761513 100644
>> --- a/arch/arm64/kernel/psci.c
>> +++ b/arch/arm64/kernel/psci.c
>> @@ -70,6 +70,7 @@ enum psci_function {
>> static DEFINE_PER_CPU_READ_MOSTLY(struct psci_power_state *, psci_power_state);
>>
>> static u32 psci_function_id[PSCI_FN_MAX];
>> +static u32 cpu_psci_idle_state_count;
>>
>> static int psci_to_linux_errno(int errno)
>> {
>> @@ -239,6 +240,8 @@ static int __maybe_unused cpu_psci_cpu_init_idle(struct device_node *cpu_node,
>> if (!count)
>> return -ENODEV;
>>
>> + cpu_psci_idle_state_count = count;
>> +
>> psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
>> if (!psci_states)
>> return -ENOMEM;
>> @@ -521,6 +524,11 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
>> {
>> int ret;
>> struct psci_power_state *state = __get_cpu_var(psci_power_state);
>> +
>> + /* Check and select the deepest idle state */
>> + if (index == CPU_PSCI_IDLE_INDEX_MAX)
>> + index = cpu_psci_idle_state_count;
>> +
>> /*
>> * idle state index 0 corresponds to wfi, should never be called
>> * from the cpu_suspend operations.
>> --
>> 1.9.1
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index
2014-10-30 10:19 ` Lorenzo Pieralisi
@ 2014-10-30 11:31 ` amit daniel kachhap
0 siblings, 0 replies; 11+ messages in thread
From: amit daniel kachhap @ 2014-10-30 11:31 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Catalin Marinas, Mark Rutland, Ashwin Chaugule, Vladimir Murzin,
Will Deacon
On Thu, Oct 30, 2014 at 3:49 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> Hi Amit,
>
> On Thu, Oct 30, 2014 at 03:55:37AM +0000, Amit Daniel Kachhap wrote:
>> This fix rectifies the psci cpu_suspend to check the C-state type
>> corresponding to the requested index.
>
> Can you reword the commit log please ?
>
> "This fix rectifies the psci cpu_suspend implementation to check the
> PSCI power state parameter type field associated with the requested idle
> state index."
>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>> arch/arm64/kernel/psci.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
>> index 2178d6e..4ebc146 100644
>> --- a/arch/arm64/kernel/psci.c
>> +++ b/arch/arm64/kernel/psci.c
>> @@ -530,7 +530,7 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
>> if (WARN_ON_ONCE(!index || !state))
>> return -EINVAL;
>>
>> - if (state->type == PSCI_POWER_STATE_TYPE_STANDBY)
>> + if (state[index - 1].type == PSCI_POWER_STATE_TYPE_STANDBY)
>> ret = psci_ops.cpu_suspend(state[index - 1], 0);
>> else
>> ret = __cpu_suspend(index, psci_suspend_finisher);
>> --
>> 1.9.1
>
> Can you resend it as a single patch (ie this patch does not apply if
> patch 1 is missing, and I am not sure we need patch 1) and send it to
> Catalin and Will to merge it asap ?
Ok sure.
>
> Please add my:
>
> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
> Thanks,
> Lorenzo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised
2014-10-30 10:29 ` [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised Lorenzo Pieralisi
@ 2014-10-30 11:35 ` Amit Kachhap
0 siblings, 0 replies; 11+ messages in thread
From: Amit Kachhap @ 2014-10-30 11:35 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Catalin Marinas, Mark Rutland, Ashwin Chaugule, Vladimir Murzin
On Thu, Oct 30, 2014 at 3:59 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Thu, Oct 30, 2014 at 03:55:36AM +0000, Amit Daniel Kachhap wrote:
>> Without this cpu_suspend may cause crash dump when psci cpuidle
>> is not initialised and cpu_suspend is called.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>> arch/arm64/kernel/psci.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
>> index 866c1c8..2178d6e 100644
>> --- a/arch/arm64/kernel/psci.c
>> +++ b/arch/arm64/kernel/psci.c
>> @@ -523,9 +523,11 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
>> struct psci_power_state *state = __get_cpu_var(psci_power_state);
>> /*
>> * idle state index 0 corresponds to wfi, should never be called
>> - * from the cpu_suspend operations
>> + * from the cpu_suspend operations.
>> + * Also psci_power_state variable should have been populated by
>> + * above init idle routine.
>> */
>> - if (WARN_ON_ONCE(!index))
>> + if (WARN_ON_ONCE(!index || !state))
>> return -EINVAL;
>
> I thought about this when developing the code, at the moment this can't
> happen so I did not add any check for that. I would wait for the dust
> to settle on the API usage (and if we need it for something different
> than idle) before adding checks that might turn out useless, so we will
> keep this patch on the back-burner for now.
Ok right this fix is not mandatory.
>
> Thanks,
> Lorenzo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.
2014-10-30 10:46 ` Lorenzo Pieralisi
2014-10-30 11:22 ` amit daniel kachhap
@ 2014-12-01 8:08 ` amit daniel kachhap
2014-12-15 18:34 ` Lorenzo Pieralisi
1 sibling, 1 reply; 11+ messages in thread
From: amit daniel kachhap @ 2014-12-01 8:08 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Catalin Marinas, Mark Rutland, Ashwin Chaugule, Vladimir Murzin,
Charles Garcia-Tobin
On Thu, Oct 30, 2014 at 4:16 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Thu, Oct 30, 2014 at 03:55:38AM +0000, Amit Daniel Kachhap wrote:
>> This feature will be useful for system suspend/resume which may
>> intend to invoke cpu_suspend with deepest possible C state level.
>
> We are working on defining system states in the PSCI specification,
> but it is not set in stone yet (Cc'ed Charles).
>
> I guess you are posting this code because you have already an
> implementation that supports system suspend/resume, so I am
> taking an immediate action to come up with an API suitable
> for that asap.
Hi Lorenzo,
Any update on this API for system sleep.
Regards,
Amit
>
> On a side note I do not necessarily like this magic idle index
> logic, I prefer adding an API for that, that translates the call
> into whatever is required by the back-end (ie PSCI) implementation.
>
> Thanks,
> Lorenzo
>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>> arch/arm64/include/asm/psci.h | 2 ++
>> arch/arm64/kernel/psci.c | 8 ++++++++
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h
>> index e5312ea..487e52b 100644
>> --- a/arch/arm64/include/asm/psci.h
>> +++ b/arch/arm64/include/asm/psci.h
>> @@ -14,6 +14,8 @@
>> #ifndef __ASM_PSCI_H
>> #define __ASM_PSCI_H
>>
>> +#define CPU_PSCI_IDLE_INDEX_MAX 0xff
>> +
>> int psci_init(void);
>>
>> #endif /* __ASM_PSCI_H */
>> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
>> index 4ebc146..a761513 100644
>> --- a/arch/arm64/kernel/psci.c
>> +++ b/arch/arm64/kernel/psci.c
>> @@ -70,6 +70,7 @@ enum psci_function {
>> static DEFINE_PER_CPU_READ_MOSTLY(struct psci_power_state *, psci_power_state);
>>
>> static u32 psci_function_id[PSCI_FN_MAX];
>> +static u32 cpu_psci_idle_state_count;
>>
>> static int psci_to_linux_errno(int errno)
>> {
>> @@ -239,6 +240,8 @@ static int __maybe_unused cpu_psci_cpu_init_idle(struct device_node *cpu_node,
>> if (!count)
>> return -ENODEV;
>>
>> + cpu_psci_idle_state_count = count;
>> +
>> psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
>> if (!psci_states)
>> return -ENOMEM;
>> @@ -521,6 +524,11 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
>> {
>> int ret;
>> struct psci_power_state *state = __get_cpu_var(psci_power_state);
>> +
>> + /* Check and select the deepest idle state */
>> + if (index == CPU_PSCI_IDLE_INDEX_MAX)
>> + index = cpu_psci_idle_state_count;
>> +
>> /*
>> * idle state index 0 corresponds to wfi, should never be called
>> * from the cpu_suspend operations.
>> --
>> 1.9.1
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.
2014-12-01 8:08 ` amit daniel kachhap
@ 2014-12-15 18:34 ` Lorenzo Pieralisi
0 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Pieralisi @ 2014-12-15 18:34 UTC (permalink / raw)
To: amit daniel kachhap
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Catalin Marinas, Mark Rutland, Ashwin Chaugule, Vladimir Murzin,
Charles Garcia-Tobin
On Mon, Dec 01, 2014 at 08:08:55AM +0000, amit daniel kachhap wrote:
> On Thu, Oct 30, 2014 at 4:16 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On Thu, Oct 30, 2014 at 03:55:38AM +0000, Amit Daniel Kachhap wrote:
> >> This feature will be useful for system suspend/resume which may
> >> intend to invoke cpu_suspend with deepest possible C state level.
> >
> > We are working on defining system states in the PSCI specification,
> > but it is not set in stone yet (Cc'ed Charles).
> >
> > I guess you are posting this code because you have already an
> > implementation that supports system suspend/resume, so I am
> > taking an immediate action to come up with an API suitable
> > for that asap.
> Hi Lorenzo,
>
> Any update on this API for system sleep.
Hi Amit,
Sorry for the delay in replying, I am working on it at the moment, we will
post the API in the coming weeks.
Thank you,
Lorenzo
>
> Regards,
> Amit
>
> >
> > On a side note I do not necessarily like this magic idle index
> > logic, I prefer adding an API for that, that translates the call
> > into whatever is required by the back-end (ie PSCI) implementation.
> >
> > Thanks,
> > Lorenzo
> >
> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >> ---
> >> arch/arm64/include/asm/psci.h | 2 ++
> >> arch/arm64/kernel/psci.c | 8 ++++++++
> >> 2 files changed, 10 insertions(+)
> >>
> >> diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h
> >> index e5312ea..487e52b 100644
> >> --- a/arch/arm64/include/asm/psci.h
> >> +++ b/arch/arm64/include/asm/psci.h
> >> @@ -14,6 +14,8 @@
> >> #ifndef __ASM_PSCI_H
> >> #define __ASM_PSCI_H
> >>
> >> +#define CPU_PSCI_IDLE_INDEX_MAX 0xff
> >> +
> >> int psci_init(void);
> >>
> >> #endif /* __ASM_PSCI_H */
> >> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> >> index 4ebc146..a761513 100644
> >> --- a/arch/arm64/kernel/psci.c
> >> +++ b/arch/arm64/kernel/psci.c
> >> @@ -70,6 +70,7 @@ enum psci_function {
> >> static DEFINE_PER_CPU_READ_MOSTLY(struct psci_power_state *, psci_power_state);
> >>
> >> static u32 psci_function_id[PSCI_FN_MAX];
> >> +static u32 cpu_psci_idle_state_count;
> >>
> >> static int psci_to_linux_errno(int errno)
> >> {
> >> @@ -239,6 +240,8 @@ static int __maybe_unused cpu_psci_cpu_init_idle(struct device_node *cpu_node,
> >> if (!count)
> >> return -ENODEV;
> >>
> >> + cpu_psci_idle_state_count = count;
> >> +
> >> psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL);
> >> if (!psci_states)
> >> return -ENOMEM;
> >> @@ -521,6 +524,11 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
> >> {
> >> int ret;
> >> struct psci_power_state *state = __get_cpu_var(psci_power_state);
> >> +
> >> + /* Check and select the deepest idle state */
> >> + if (index == CPU_PSCI_IDLE_INDEX_MAX)
> >> + index = cpu_psci_idle_state_count;
> >> +
> >> /*
> >> * idle state index 0 corresponds to wfi, should never be called
> >> * from the cpu_suspend operations.
> >> --
> >> 1.9.1
> >>
> >>
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-12-15 18:34 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-30 3:55 [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised Amit Daniel Kachhap
2014-10-30 3:55 ` [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index Amit Daniel Kachhap
2014-10-30 10:19 ` Lorenzo Pieralisi
2014-10-30 11:31 ` amit daniel kachhap
2014-10-30 3:55 ` [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state Amit Daniel Kachhap
2014-10-30 10:46 ` Lorenzo Pieralisi
2014-10-30 11:22 ` amit daniel kachhap
2014-12-01 8:08 ` amit daniel kachhap
2014-12-15 18:34 ` Lorenzo Pieralisi
2014-10-30 10:29 ` [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised Lorenzo Pieralisi
2014-10-30 11:35 ` Amit Kachhap
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).