Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48083 )
Change subject: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG@9 PS5, Line 9: Set mainboard_ops.init directly instead of using the indirection via a : mainboard_enable function.
I think having two different functions named init is confusing, so if anyone want to add some form of mainboard init that requires devices to be enumarated, which is generally the case for ramstage hardware init, he would be in trouble. So I'd recommend keeping this in the device_operations init even if it is not strictly needed for pcr stuff.
What two different functions?
device_operations.init and chip_operations.init
so if anyone want to add some form of mainboard init that requires devices to be enumarated, which is generally the case for ramstage hardware init, he would be in trouble.
Why would anyone be in trouble? One has to use the right functions for things, as always
Yes and device_operations.init is the best place to put hardware init things in.
So I'd recommend keeping this in the device_operations init even if it is not strictly needed for pcr stuff.
What would we do in chip init for example?
Things that you want to happen before enumeration/allocation. So for instance this is used to add chipset reserved resources like dram which the allocator cannot freely use (that cannot be done with a PCI driver). Read things from blob output and patch up the devicetree linked list is also something that is done iirc. But generally speaking you want to avoid hardware init if possible there.