Jeff Chase has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36791 )
Change subject: mb/google/fizz: Add Endeavour variant ......................................................................
Patch Set 4:
(9 comments)
Squashed changes into this one, updated copyrights, resolved gpio tables
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: […]
Done
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
Looks like this file is pretty much the same as `gpio.c` of the baseboard. […]
I was trying make it easier to review what changed from the baseboard but I should have just used another patchset. Squashed.
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... PS3, Line 237: early_gpio_table
Same question as above.
Done
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... PS3, Line 258: __weak
Why is this weak if it is being provided by the variant?
Done
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... PS3, Line 264: __weak
same here
Done
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... PS3, Line 271: cros_gpios
If baseboard is providing the same table as this, you probably don't need a separate table in the va […]
Done
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
I found some more already in the tree. I'll push a change taking care of them.
Done
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
This file as a whole also seems to come from the baseboard, rather than Karma.
Done
https://review.coreboot.org/c/coreboot/+/36791/3/src/mainboard/google/fizz/v... PS3, Line 29: __weak
same here.
Done