On Thu, Sep 26, 2024 at 11:28 PM Simon Glass <s...@chromium.org> wrote:
>
> Hi Patrick,
>
> On Thu, 26 Sept 2024 at 10:03, Patrick Rudolph
> <patrick.rudo...@9elements.com> wrote:
> >
> > Add support for Arm sbsa [1] v0.3+ that is supported by QEMU [2].
> >
> > Unlike other Arm based platforms the machine only provides a minimal
> > FDT that contains number of CPUs, ammount of memory and machine-version.
> > The boot firmware has to provide ACPI tables to the OS.
> > Due to this design a full DTB is added here as well that allows U-Boot's
> > driver to properly function. The DTB is appended at the end of the U-Boot
> > image and will be merged with the QEMU provided DTB.
> >
> > In addition provide documentation how to use, enable binman to fabricate 
> > both
> > ROMs that are required to boot and add ACPI tables to make it full 
> > compatible
> > to the EDK2 reference implementation.
> >
> > The board was tested using Fedora 40 Aarch64 Workstation. It's able
> > to boot from USB and AHCI or network.
> >
> > Tested and found working:
> > - serial
> > - PCI
> > - xHCI
> > - Bochs display
> > - AHCI
> > - network using e1000e
> > - CPU init
> > - Booting Fedora 40
> >
> > 1: Server Base System Architecture (SBSA)
> > 2: https://www.qemu.org/docs/master/system/arm/sbsa.html
> >
> > Signed-off-by: Patrick Rudolph <patrick.rudo...@9elements.com>
> > Cc: Peter Robinson <pbrobin...@gmail.com>
> > Cc: Simon Glass <s...@chromium.org>
> > Cc: Tom Rini <tr...@konsulko.com>
> > ---
> > Changelog v3:
> > - Add GIC and GIC-ITS to devicetree
> > - Select GICv3 driver
> > - Drop acpi_fill_madt and use driver model instead
> > Changelog v4:
> > - Drop CPU platform code
> > - Enhance the DT to allow MADT generation from DT
> > Changelog v5:
> > - Add full DT and place it at the end of U-Boot
> > - Merge DT with QEMU's DT
> > - Drop DT generation code
> > - Fix flash region length
> > - Drop enable_caches()
> > - Support platforms that do not pass FDT in x0
> > ---
> >  arch/arm/Kconfig                            |   3 +-
> >  arch/arm/dts/qemu-sbsa.dts                  | 138 ++++++
> >  arch/arm/include/asm/arch-qemu-sbsa/boot0.h |  34 ++
> >  arch/arm/mach-qemu/Kconfig                  |  36 +-
> >  board/emulation/qemu-arm/MAINTAINERS        |   2 +
> >  board/emulation/qemu-sbsa/Kconfig           |  59 +++
> >  board/emulation/qemu-sbsa/Makefile          |   8 +
> >  board/emulation/qemu-sbsa/acpi.c            | 193 ++++++++
> >  board/emulation/qemu-sbsa/dsdt.asl          | 483 ++++++++++++++++++++
> >  board/emulation/qemu-sbsa/lowlevel_init.S   |  22 +
> >  board/emulation/qemu-sbsa/qemu-sbsa.c       | 312 +++++++++++++
> >  board/emulation/qemu-sbsa/qemu-sbsa.env     |  14 +
> >  board/emulation/qemu-sbsa/qemu-sbsa.h       |  38 ++
> >  board/emulation/qemu-sbsa/smc.c             |  72 +++
> >  configs/qemu-arm-sbsa_defconfig             |  10 +
> >  doc/board/emulation/index.rst               |   1 +
> >  doc/board/emulation/qemu-sbsa.rst           |  98 ++++
> >  doc/develop/driver-model/virtio.rst         |   1 +
> >  include/configs/qemu-sbsa.h                 | 104 +++++
> >  19 files changed, 1621 insertions(+), 7 deletions(-)
> >  create mode 100644 arch/arm/dts/qemu-sbsa.dts
> >  create mode 100644 arch/arm/include/asm/arch-qemu-sbsa/boot0.h
> >  create mode 100644 board/emulation/qemu-sbsa/Kconfig
> >  create mode 100644 board/emulation/qemu-sbsa/Makefile
> >  create mode 100644 board/emulation/qemu-sbsa/acpi.c
> >  create mode 100644 board/emulation/qemu-sbsa/dsdt.asl
> >  create mode 100644 board/emulation/qemu-sbsa/lowlevel_init.S
> >  create mode 100644 board/emulation/qemu-sbsa/qemu-sbsa.c
> >  create mode 100644 board/emulation/qemu-sbsa/qemu-sbsa.env
> >  create mode 100644 board/emulation/qemu-sbsa/qemu-sbsa.h
> >  create mode 100644 board/emulation/qemu-sbsa/smc.c
> >  create mode 100644 configs/qemu-arm-sbsa_defconfig
> >  create mode 100644 doc/board/emulation/qemu-sbsa.rst
> >  create mode 100644 include/configs/qemu-sbsa.h
>
> Please check the header order throughout
> E.g. board/emulation/qemu-sbsa/qemu-sbsa.c is still incorrect
>
> You should not need to include 'config.h' as kconfig is included by default?
>
> Do you need to add all the CFG items? Are they present in the devicetree?
What are CFG items? Do you refer to Kconfig options being selected or modified?
The Kconfig options selected allow to use all of the devices present in the DT.

It looks like on some U-Boot boards most Kconfigs are not selected,
leaving you with a
non booting default configuration. Only the *_defconfig found in
configs will generate a
"usable" image, which I find kind of strange.

>
> For Binman the default pad-byte is 0, so you can drop those lines.
>
> Reviewed-by: Simon Glass <s...@chromium.org>
>
> Regards,
> Simon

Reply via email to