Hi Conor, On Mon, Jul 3, 2023 at 8:20 PM Conor Dooley <conor.dooley@xxxxxxxxxxxxx> wrote: > > 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. Okay, other arches may use FFI in the future. Keep the code as-is seems. > > 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). Great, I have to learn from it. > > > > > > +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". Oh, that's what it means, okay, I'll update it on v3. > > Thanks, > Conor. Thanks, Yunhui
