Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36791 )
Change subject: google/endeavour: add variant based on karma ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36791/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36791/3//COMMIT_MSG@7 PS3, Line 7: google/endeavour: add variant based on karma Suggestion: Mention the baseboard and the new variant's name in the commit summary:
mb/google/fizz: Add Endeavour variant
Use the already-existing Karma variant as a base.
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... File src/mainboard/google/fizz/variants/endeavour/gpio.c:
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... PS3, Line 22: gpio_table
Is this table same as the baseboard? Or is there any difference?
Looks like this file is pretty much the same as `gpio.c` of the baseboard. Maybe the existing Karma variant has a better default.
Looks like the correct GPIOs are added on CB:36792. IMHO, it would be better to squash that change into this one, so that this can be boot-tested.
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... File src/mainboard/google/fizz/variants/endeavour/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... PS3, Line 4: Inc
This should be changed to LLC in all the copyright headers.
I found some more already in the tree. I'll push a change taking care of them.
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... File src/mainboard/google/fizz/variants/endeavour/nhlt.c:
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... PS3, Line 20: __weak
weak not required in variant specific file.
This file as a whole also seems to come from the baseboard, rather than Karma.