Re: [External] Re: [PATCH v2 1/3] riscv: obtain ACPI RSDP from FFI.

[Conor Dooley <conor.dooley@microchip.com>]

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

On Mon, Jul 03, 2023 at 06:16:07PM +0800, 运辉崔 wrote:
> Hi Conor,

> This needs to be confirmed with you:

> Continue to follow the current code structure, patch 1/3 is placed in
> arch/riscv/, and 2/3 is placed under driver/firmware?

You do want the SMBIOS stuff to be cross architecture, right?
If so, keeping the code as-is seems to make the most sense to me.

> How about changing the commit log to the following?
> 
> riscv: obtain ACPI RSDP from devicetree.
> 
> On RISC-V, when using Coreboot to start, since Coreboot only supports
> DTS but not EFI, and
> RISC-V does not have a reserved address segment.
> When the system enables ACPI, ACPI RSDP needs to be passed through DTS

I would probably write something like:
	On RISC-V, Coreboot does not support booting using EFI, only devicetree
	nor does RISC-V have a reserved address segment.
	To allow using Coreboot on platforms that require ACPI, the ACPI RSDP
	needs to be passed to supervisor mode software using devicetree.
	Add support for parsing the "configtbls" devicetree node to find the
	ACPI entry point and use wire up acpi_arch_get_root_pointer().
	This feature is known as FDT Firmware Interface (FFI).

> > > > > +extern u64 acpi_rsdp;
> > > >
> > > > /stuff/linux/drivers/acpi/osl.c:178:22: error: redefinition of 'acpi_rsdp' with a different type: 'unsigned long' vs 'u64' (aka 'unsigned long long')
> > > >
> > > > Fails to build when Kexec is enabled.
> > >
> > > Rename my acpi_rsdp to arch_acpi_rsdp? WDYT?
> >
> > You could do s/arch/riscv/ either, that'd match what we prefix a lot of
> > stuff with.
> 
>  Sorry, I don't quite understand what you mean. Could you tell me in detail?

What I meant is that variables & functions in /arch/riscv are often
prefixed with riscv_. I was saying that you could change "arch_acpi_rsdp"
to "riscv_acpi_rsdp".

Thanks,
Conor.

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