Re: [PATCH V4 19/23] RISC-V: Add ACPI initialization in setup_arch()

Conor Dooley
2023-04-04T21:39:04Z

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

On Tue, Apr 04, 2023 at 11:50:33PM +0530, Sunil V L wrote:
> Initialize the ACPI core for RISC-V during boot.
>
> ACPI tables and interpreter are initialized based on
> the information passed from the firmware and the value of
> the kernel parameter 'acpi'.
>
> With ACPI support added for RISC-V, the kernel parameter 'acpi'
> is also supported on RISC-V. Hence, update the documentation.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

> + /* Parse the ACPI tables for possible boot-time configuration */
> + acpi_boot_table_init();
> + if (acpi_disabled) {
> + if (IS_ENABLED(CONFIG_BUILTIN_DTB)) {
> + unflatten_and_copy_device_tree();
> + } else {
> + if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa))))
> + unflatten_device_tree();
> + else
> + pr_err("No DTB found in kernel mappings\n");
> + }
> + } else {
> + early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa)));

I'm probably forgetting something, but this seems very non-obvious to
me:
Why are you running early_init_dt_verify() when ACPI is enabled?
I think that one deserves a comment so that next time someone looks at
this (that doesn't live in ACPI land) they've know exactly why this is
like it is.

Doubly so since this is likely to change with some of Alex's bits moving
the dtb back into the fixmap.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]