Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39857 )
Change subject: mb/google/volteer: Create Malefor variant ......................................................................
Patch Set 5:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39857/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/malefor/gpio.c:
https://review.coreboot.org/c/coreboot/+/39857/5/src/mainboard/google/voltee... PS5, Line 10: gpio_table Is this gpio_table just a complete copy of ripto/volteer? Or is this updated to handle malefor? If it is just a copy, can we just rely on using the baseboard default definitions instead of copying these?
https://review.coreboot.org/c/coreboot/+/39857/5/src/mainboard/google/voltee... PS5, Line 405: early_gpio_table Same comment as above.
https://review.coreboot.org/c/coreboot/+/39857/5/src/mainboard/google/voltee... PS5, Line 446: cros_gpios Same comment as above.
https://review.coreboot.org/c/coreboot/+/39857/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/malefor/memory.c:
https://review.coreboot.org/c/coreboot/+/39857/5/src/mainboard/google/voltee... PS5, Line 8: baseboard This is not really baseboard. I think you can just name this memcfg
https://review.coreboot.org/c/coreboot/+/39857/5/src/mainboard/google/voltee... PS5, Line 39: __weak Why is this weak?
https://review.coreboot.org/c/coreboot/+/39857/5/src/mainboard/google/voltee... PS5, Line 44: int variant_memory_sku(void) : { : gpio_t spd_gpios[] = { : GPIO_MEM_CONFIG_0, : GPIO_MEM_CONFIG_1, : GPIO_MEM_CONFIG_2, : GPIO_MEM_CONFIG_3, : }; : : return gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); : } Is this different than what baseboard is providing?