Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42551 )
Change subject: hatch: Create wyvern variant ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review-2
Patch Set 1:
Patch Set 1:
(4 comments)
Hey Paul, thanks for the patch. Please see inline comments. I think there is a few not too bad bugs here however could you raise them from review and assign them to me please.
All of this code comes from the templates, see util/mainboard/google/puff/template. That's where we'll need to make any source code changes for future variants.
OK, however I think we should address some key issues here like puff variants not using hardcoded spd data. Then regenerate this patch, otherwise we are essentially merging an unbootable board into coreboot.
What's the time frame for getting this addressed?
I think we can probably address these issues in pretty short succession, just three main issues missing and a #include fix would make the generated board viable.
Right now, Wyvern firmware is only supposed to be a copy of Puff, with the name Puff scratched out and Wyvern written over it. So Wyvern firmware today should be bootable on Puff hardware. That's the justification for letting this through and *then* making the other changes you've proposed.
I understand the objective, the issue is that this isn't a copy of Puff as it is missing all the configuration details of mainboard.c and gpio.c . I suppose we should copy those into the template directory and a copy of each be generated with this board. The ec.h commit should land and we should align on that as well and finally a copy of Puff's devicetree overridetree.cb should perhaps be landed in the template directory.
Patch series created to resolve these issues: https://review.coreboot.org/c/coreboot/+/42672