[03/19] ARM64 / ACPI: Introduce lowlevel suspend function

Message ID 1406206825-15590-4-git-send-email-hanjun.guo@linaro.org
State New
Headers show

Commit Message

Hanjun Guo July 24, 2014, 1 p.m.
From: Graeme Gregory <graeme.gregory@linaro.org>

acpi_wakeup_address is used on x86 as the address bios jumps into
when machine wakes up from suspend. As arm64 does not have such a
bios this mechanism will be provided by other means. But the define
is still required inside the acpi core.

Introduce a null stub for acpi_suspend_lowlevel as this is also
required by core. This will be filled in when standards are
defined for arm64 ACPI global power states.

Reviewed-by: Grant Likely <grant.likely@linaro.org>
Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 arch/arm64/include/asm/acpi.h |   12 ++++++++++++
 arch/arm64/kernel/acpi.c      |    7 +++++++
 2 files changed, 19 insertions(+)

Comments

Mark Rutland July 24, 2014, 3:34 p.m. | #1
On Thu, Jul 24, 2014 at 02:00:09PM +0100, Hanjun Guo wrote:
> From: Graeme Gregory <graeme.gregory@linaro.org>
> 
> acpi_wakeup_address is used on x86 as the address bios jumps into
> when machine wakes up from suspend. As arm64 does not have such a
> bios this mechanism will be provided by other means. But the define
> is still required inside the acpi core.
> 
> Introduce a null stub for acpi_suspend_lowlevel as this is also
> required by core. This will be filled in when standards are
> defined for arm64 ACPI global power states.
> 
> Reviewed-by: Grant Likely <grant.likely@linaro.org>
> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
>  arch/arm64/include/asm/acpi.h |   12 ++++++++++++
>  arch/arm64/kernel/acpi.c      |    7 +++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index e8581ea..44b617f 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -36,6 +36,18 @@ static inline bool acpi_has_cpu_in_madt(void)
>  	return 1;
>  }
>  
> +/* Low-level suspend routine.
> + *
> + * ACPI S-states for ARM64 have to be defined
> + * and approved before doing anything else, maybe
> + * we need update the ACPI spec, here we
> + * just introduce function and macro needed by
> + * ACPI core as IA64 did, and revisit them when
> + * the spec is ready.
> + */
> +extern int (*acpi_suspend_lowlevel)(void);
> +#define acpi_wakeup_address 0
> +
>  #endif /* CONFIG_ACPI */
>  
>  #endif /*_ASM_ACPI_H*/
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index ac78e4e..f5a10b5 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -95,3 +95,10 @@ static int __init parse_acpi(char *arg)
>  	return 0;
>  }
>  early_param("acpi", parse_acpi);
> +
> +/*
> + * acpi_suspend_lowlevel() - save kernel state and suspend.
> + *
> + * TBD when ARM/ARM64 starts to support suspend...
> + */
> +int (*acpi_suspend_lowlevel)(void);

Shouldn't this should be intialised to NULL explicitly?

Thanks,
Mark.
Hanjun Guo July 25, 2014, 10:42 a.m. | #2
On 2014-7-24 23:34, Mark Rutland wrote:
[...]
>>  early_param("acpi", parse_acpi);
>> +
>> +/*
>> + * acpi_suspend_lowlevel() - save kernel state and suspend.
>> + *
>> + * TBD when ARM/ARM64 starts to support suspend...
>> + */
>> +int (*acpi_suspend_lowlevel)(void);
> 
> Shouldn't this should be intialised to NULL explicitly?

Yes, as ARM64 does not support suspend, initialize it
to NULL will be fine, will update in next version.

Thanks
Hanjun
Sudeep Holla July 28, 2014, 6:28 p.m. | #3
On 24/07/14 14:00, Hanjun Guo wrote:
> From: Graeme Gregory <graeme.gregory@linaro.org>
>
> acpi_wakeup_address is used on x86 as the address bios jumps into
> when machine wakes up from suspend. As arm64 does not have such a
> bios this mechanism will be provided by other means. But the define
> is still required inside the acpi core.
>
> Introduce a null stub for acpi_suspend_lowlevel as this is also
> required by core. This will be filled in when standards are
> defined for arm64 ACPI global power states.
>
> Reviewed-by: Grant Likely <grant.likely@linaro.org>
> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
>   arch/arm64/include/asm/acpi.h |   12 ++++++++++++
>   arch/arm64/kernel/acpi.c      |    7 +++++++
>   2 files changed, 19 insertions(+)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index e8581ea..44b617f 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -36,6 +36,18 @@ static inline bool acpi_has_cpu_in_madt(void)
>   	return 1;
>   }
>
> +/* Low-level suspend routine.
> + *
> + * ACPI S-states for ARM64 have to be defined
> + * and approved before doing anything else, maybe
> + * we need update the ACPI spec, here we
> + * just introduce function and macro needed by
> + * ACPI core as IA64 did, and revisit them when
> + * the spec is ready.
> + */
> +extern int (*acpi_suspend_lowlevel)(void);
> +#define acpi_wakeup_address 0
> +

While I understand that this is temporary solution, but will this be
sufficient to make sure we don't enter acpi_suspend_enter ?

A brief look at acpi_suspend_enter showed access to write to
ACPI_BITREG_SCI_ENABLE which might just explode. But if you are sure that
it will not be executed, then it should be fine for now.

Regards,
Sudeep


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hanjun Guo July 29, 2014, 1 p.m. | #4
On 2014-7-29 2:28, Sudeep Holla wrote:
> 
> 
> On 24/07/14 14:00, Hanjun Guo wrote:
>> From: Graeme Gregory <graeme.gregory@linaro.org>
>>
>> acpi_wakeup_address is used on x86 as the address bios jumps into
>> when machine wakes up from suspend. As arm64 does not have such a
>> bios this mechanism will be provided by other means. But the define
>> is still required inside the acpi core.
>>
>> Introduce a null stub for acpi_suspend_lowlevel as this is also
>> required by core. This will be filled in when standards are
>> defined for arm64 ACPI global power states.
>>
>> Reviewed-by: Grant Likely <grant.likely@linaro.org>
>> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> ---
>>   arch/arm64/include/asm/acpi.h |   12 ++++++++++++
>>   arch/arm64/kernel/acpi.c      |    7 +++++++
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index e8581ea..44b617f 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -36,6 +36,18 @@ static inline bool acpi_has_cpu_in_madt(void)
>>       return 1;
>>   }
>>
>> +/* Low-level suspend routine.
>> + *
>> + * ACPI S-states for ARM64 have to be defined
>> + * and approved before doing anything else, maybe
>> + * we need update the ACPI spec, here we
>> + * just introduce function and macro needed by
>> + * ACPI core as IA64 did, and revisit them when
>> + * the spec is ready.
>> + */
>> +extern int (*acpi_suspend_lowlevel)(void);
>> +#define acpi_wakeup_address 0
>> +
> 
> While I understand that this is temporary solution, but will this be
> sufficient to make sure we don't enter acpi_suspend_enter ?
> 
> A brief look at acpi_suspend_enter showed access to write to
> ACPI_BITREG_SCI_ENABLE which might just explode. But if you are sure that
> it will not be executed, then it should be fine for now.

I think it will not be executed, since ARM64 do not support S1 and S3 in ACPI
for now, and that state will not defined, then acpi_suspend_enter will not be
called.

Thanks
Hanjun

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index e8581ea..44b617f 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -36,6 +36,18 @@  static inline bool acpi_has_cpu_in_madt(void)
 	return 1;
 }
 
+/* Low-level suspend routine.
+ *
+ * ACPI S-states for ARM64 have to be defined
+ * and approved before doing anything else, maybe
+ * we need update the ACPI spec, here we
+ * just introduce function and macro needed by
+ * ACPI core as IA64 did, and revisit them when
+ * the spec is ready.
+ */
+extern int (*acpi_suspend_lowlevel)(void);
+#define acpi_wakeup_address 0
+
 #endif /* CONFIG_ACPI */
 
 #endif /*_ASM_ACPI_H*/
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index ac78e4e..f5a10b5 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -95,3 +95,10 @@  static int __init parse_acpi(char *arg)
 	return 0;
 }
 early_param("acpi", parse_acpi);
+
+/*
+ * acpi_suspend_lowlevel() - save kernel state and suspend.
+ *
+ * TBD when ARM/ARM64 starts to support suspend...
+ */
+int (*acpi_suspend_lowlevel)(void);