[U-Boot] [PATCH 02/22] x86: acpi: Add Kconfig option and header file for ACPI resume

[Simon Glass]

Hi Bin,

On 16 March 2017 at 08:26, Bin Meng <bmeng.cn at gmail.com> wrote:
> This introduces a Kconfig option for ACPI S3 resume, as well as a
> header file to include anything related to ACPI S3 resume.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>
>  arch/x86/Kconfig               | 12 +++++++++
>  arch/x86/include/asm/acpi_s3.h | 58 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+)
>  create mode 100644 arch/x86/include/asm/acpi_s3.h

Reviewed-by: Simon Glass <sjg at chromium.org>

A few nits below

>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index dfdd756..7ea9148 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -571,6 +571,18 @@ config GENERATE_ACPI_TABLE
>
>  endmenu
>
> +config HAVE_ACPI_RESUME
> +       bool "Enable ACPI S3 resume"
> +       help
> +         Select this to enable ACPI S3 resume. S3 is an ACPI defined sleeping

ACPI-defined

> +         state where all system context is lost except system memory. U-Boot
> +         is responsible for restore the machine state as it is before sleep.

restoring

as it was before

> +         It needs restore the memory controller, not overwriting memory which

s/not/without/ ?

> +         is not marked as reserved. For the peripherals which lose their
> +         registers, U-Boot needs to write the original value. When everything
> +         is done, U-Boot needs to find out the wakeup vector provided by OSes
> +         and jump there.
> +
>  config MAX_PIRQ_LINKS
>         int
>         default 8
> diff --git a/arch/x86/include/asm/acpi_s3.h b/arch/x86/include/asm/acpi_s3.h
> new file mode 100644
> index 0000000..6fbfc3e
> --- /dev/null
> +++ b/arch/x86/include/asm/acpi_s3.h
> @@ -0,0 +1,58 @@
> +/*
> + * Copyright (C) 2017, Bin Meng <bmeng.cn at gmail.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef __ASM_ACPI_S3_H__
> +#define __ASM_ACPI_S3_H__
> +
> +/* PM1_STATUS register */
> +#define WAK_STS                (1 << 15)
> +#define PCIEXPWAK_STS  (1 << 14)
> +#define RTC_STS                (1 << 10)
> +#define SLPBTN_STS     (1 << 9)
> +#define PWRBTN_STS     (1 << 8)
> +#define GBL_STS                (1 << 5)
> +#define BM_STS         (1 << 4)
> +#define TMR_STS                (1 << 0)
> +
> +/* PM1_CNT register */
> +#define SLP_EN         (1 << 13)
> +#define SLP_TYP_SHIFT  10
> +#define SLP_TYP                (7 << SLP_TYP_SHIFT)
> +#define SLP_TYP_S0     0
> +#define SLP_TYP_S1     1
> +#define SLP_TYP_S3     5
> +#define SLP_TYP_S4     6
> +#define SLP_TYP_S5     7
> +
> +enum acpi_sleep_state {
> +       ACPI_S0,
> +       ACPI_S1,
> +       ACPI_S2,
> +       ACPI_S3,
> +       ACPI_S4,
> +       ACPI_S5,
> +};
> +
> +/* Given the provided PM1 control register return the ACPI sleep type */

@return ...

Does this need to be inline?

> +static inline enum acpi_sleep_state acpi_sleep_from_pm1(u32 pm1_cnt)
> +{
> +       switch ((pm1_cnt & SLP_TYP) >> SLP_TYP_SHIFT) {
> +       case SLP_TYP_S0:
> +               return ACPI_S0;
> +       case SLP_TYP_S1:
> +               return ACPI_S1;
> +       case SLP_TYP_S3:
> +               return ACPI_S3;
> +       case SLP_TYP_S4:
> +               return ACPI_S4;
> +       case SLP_TYP_S5:
> +               return ACPI_S5;
> +       }
> +
> +       return -1;

-EINVAL ?

> +}
> +
> +#endif /* __ASM_ACPI_S3_H__ */
> --
> 2.9.2
>

Regards,
Simon