Hi Heinrich,
On Fri, 8 Nov 2024 at 04:19, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 8. November 2024 08:10:05 MEZ schrieb Patrick Rudolph patrick.rudolph@9elements.com:
Hi Heinrich, On Thu, Nov 7, 2024 at 10:19 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 10/23/24 15:19, Patrick Rudolph wrote:
From: Maximilian Brune maximilian.brune@9elements.com
Write the FADT in common code since it's used on all architectures. Since the FADT is mandatory all SoCs or mainboards must implement the introduced function acpi_fill_fadt() and properly update the FADT.
Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-by: Simon Glass sjg@chromium.org Cc: Simon Glass sjg@chromium.org Cc: Bin Meng bmeng.cn@gmail.com
This merged patch breaks
make qemu_riscv64_defconfig acpi.config make
I'm sorry, since the CI doesn't build that target, I was not aware of build failures.
You are implementing function acpi_fill_fadt() only for x86 and call it in code that is used by all architectures.
acpi_fill_fadt() is called on all architectures that are ACPI enabled and build by the CI. There might additional ACPI enabled platforms, but since they are not build tested, they aren't officially supported, are they?
I worked on getting ACPI enabled on arm64 and riscv64 QEMU but could have made better by adding it to CI.
For some features it was decided that we prefer config overlays over deconfigs. But the test concept is kind of missing.
I am not a fan either. But I just sent a patch[1] proposing what we might do about it, assuming we are going to continue down this 'config fragment' path. It isn't a good solution, since it will potentially explode the number of builds, unfortunately. But it does have some advantages. For example we could build all boards without CONFIG_CMDLINE or filesystems, to make sure all is well.
Best regards
Heinrich
ACPI_WRITER(5fadt, "FADT", acpi_write_fadt, 0); makes no sense for CONFIG_QFW=y because QEMU is providing the table.
Good catch. I'll send a follow up series.
@Tom: After getting this fixed we should add a build with acpi.config into the CI.
further comments below:
Changelog v4:
- Drop __weak attribute
arch/sandbox/lib/acpi_table.c | 6 +++++ arch/x86/cpu/apollolake/acpi.c | 20 ++++------------ arch/x86/cpu/baytrail/acpi.c | 17 +------------- arch/x86/cpu/quark/acpi.c | 19 +--------------- arch/x86/cpu/tangier/acpi.c | 25 ++------------------ arch/x86/include/asm/acpi_table.h | 12 ---------- arch/x86/lib/acpi_table.c | 23 ------------------- include/acpi/acpi_table.h | 9 ++++++++ lib/acpi/acpi_table.c | 38 +++++++++++++++++++++++++++++++ 9 files changed, 61 insertions(+), 108 deletions(-) create mode 100644 arch/sandbox/lib/acpi_table.c
diff --git a/arch/sandbox/lib/acpi_table.c b/arch/sandbox/lib/acpi_table.c new file mode 100644 index 0000000000..10b7ce441a --- /dev/null +++ b/arch/sandbox/lib/acpi_table.c @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: GPL-2.0+ +#include <acpi/acpi_table.h>
+void acpi_fill_fadt(struct acpi_fadt *fadt) +{ +}
Regards, Simon
[1] https://patchwork.ozlabs.org/project/uboot/patch/20241108152350.3686274-9-sj...