[U-Boot] [PATCH 2/3] x86: acpi: Don't touch ACPI hardware in write_acpi_tables()

[Andy Shevchenko]

On Thu, Jun 28, 2018 at 6:38 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
> write_acpi_tables() currently touches ACPI hardware to switch to
> ACPI mode at the end. Move such operation out of this function,
> so that it only does what the function name tells us.

>  {
>         board_final_cleanup();
> +       struct acpi_fadt __maybe_unused *fadt;

I guess this would be first line in the function. No?
And personally I don't like to see __maybe_unused near to local
variables (see also below).

> +#ifdef CONFIG_HAVE_ACPI_RESUME
> +       fadt = acpi_find_fadt();
>
> -       if (fadt != NULL && gd->arch.prev_sleep_state == ACPI_S3)
> +       if (fadt && gd->arch.prev_sleep_state == ACPI_S3)
>                 acpi_resume(fadt);
>  #endif
>
>         write_tables();

> +#ifdef CONFIG_GENERATE_ACPI_TABLE
> +       fadt = acpi_find_fadt();

This sounds superfluous, we create tables ourselves, why do we need
traverse again to find them?

Looks like you would do something such as

struct acpi_fadt *fadt /* maybe save to do as well: = acpi_find_fadt() */;

...

fadt = write_tables(); //or any other way to get the pointer without traversing
...

> +
> +       /* Don't touch ACPI hardware on HW reduced platforms */
> +       if (fadt && !(fadt->flags & ACPI_FADT_HW_REDUCED_ACPI)) {
> +               /*
> +                * Other than waiting for OSPM to request us to switch to ACPI
> +                * mode, do it by ourselves, since SMI will not be triggered.
> +                */
> +               enter_acpi_mode(fadt->pm1a_cnt_blk);
> +       }


-- 
With Best Regards,
Andy Shevchenko