[PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised

[PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised

[Amit Daniel Kachhap]

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

[PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index

[Amit Daniel Kachhap]

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

Re: [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index

[Lorenzo Pieralisi]

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

Re: [PATCH 2/3] arm64: psci: fix cpu_suspend to check idle state type for index

[amit daniel kachhap]

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

[PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.

[Amit Daniel Kachhap]

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

Re: [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.

[Lorenzo Pieralisi]

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

Re: [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.

[amit daniel kachhap]

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

Re: [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.

[amit daniel kachhap]

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

Re: [PATCH 3/3] arm64: psci: Add support to call cpu_suspend with deepest C state.

[Lorenzo Pieralisi]

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
> 

Re: [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised

[Lorenzo Pieralisi]

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

Re: [PATCH 1/3] arm64: psci: warn if psci_power_state variable is not initialised

[Amit Kachhap]

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