Hi Andy,
On Thu, Jun 28, 2018 at 3:57 PM, Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Thu, Jun 28, 2018 at 6:38 AM, Bin Meng bmeng.cn@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?
Yes, it should be the first line. Will fix.
And personally I don't like to see __maybe_unused near to local variables (see also below).
Without __maybe_unused
+#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() */;
Do the assignment here only works for S3 path.
...
fadt = write_tables(); //or any other way to get the pointer without traversing
Let write_tables() return FADT address is odd. write_tables() is generic API for writing various configuration tables.
...
/* 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);}
Regards, Bin