Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33387 )
Change subject: mainboard/emulation/qemu-aarch64: Add new board for ARMv8 ......................................................................
Patch Set 26:
(3 comments)
I reverted the mainboard.c because ops->read_resources() is never called. I think the reason why read_resources isn't called is devicetree.cb is not enough but ARM systems don't rely on devicetree.cb, right?
I think this is a special case since the mainboard is the root device and src/device/device.c#read_resources() only works on the child devices of the device it was called with, so the mainboard read_resources() never gets called. I think just putting it in enable is perfectly fine, the board doesn't really use the full boot state machine anyway so nobody cares.
https://review.coreboot.org/c/coreboot/+/33387/26/Documentation/mainboard/em... File Documentation/mainboard/emulation/qemu-aarch64.md:
https://review.coreboot.org/c/coreboot/+/33387/26/Documentation/mainboard/em... PS26, Line 19: reserved for a FIT image nit: it's reserved for the kernel, technically, not the FIT image
https://review.coreboot.org/c/coreboot/+/33387/26/src/mainboard/emulation/qe... File src/mainboard/emulation/qemu-aarch64/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/33387/26/src/mainboard/emulation/qe... PS26, Line 8: chip mainboard/emulation/qemu-aarch64 I'm not an sconfig expert, but I think the mainboard is already automatically added as the root device and adding it separately here is incorrect. None of the other boards do that.
https://review.coreboot.org/c/coreboot/+/33387/26/src/mainboard/emulation/qe... PS26, Line 9: chip drivers/generic/generic # I2C0 controller Does this entry actually serve a purpose? It does nothing and means nothing, I think you should probably just remove it.