Hi Heinrich,
On Fri, 8 Nov 2024 at 04:19, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
>
> Am 8. November 2024 08:10:05 MEZ schrieb Patrick Rudolph
> <patrick.rudo...@9elements.com>:
> >Hi Heinrich,
> >On Thu, Nov 7, 2024 at 10:19 AM Heinrich Schuchardt <xypron.g...@gmx.de>
> >wrote:
> >>
> >> On 10/23/24 15:19, Patrick Rudolph wrote:
> >> > From: Maximilian Brune <maximilian.br...@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.rudo...@9elements.com>
> >> > Reviewed-by: Simon Glass <s...@chromium.org>
> >> > Cc: Simon Glass <s...@chromium.org>
> >> > Cc: Bin Meng <bmeng...@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-...@chromium.org/
