Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45686 )
Change subject: mb/prodrive/hermes: Improve board config EEPROM handling ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45686/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45686/6//COMMIT_MSG@9 PS6, Line 9: * Check and print errors returned reading from I2C Sounds like doing this in a separate commit would be very easy to review
https://review.coreboot.org/c/coreboot/+/45686/6//COMMIT_MSG@14 PS6, Line 14: * Read the UPD to disable Vtd from EEPROM : * Verify the checksum of the 'board settings' : * Enable DeepS5 depending on the configuration value in 'board settings' : * Configure USB power in S5 depending on 'board settings' : * Configure Wake On Lan depending on 'board settings' : * Configure mainboard power after G3 Each of these could be a commit
https://review.coreboot.org/c/coreboot/+/45686/6/src/mainboard/prodrive/herm... File src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h:
https://review.coreboot.org/c/coreboot/+/45686/6/src/mainboard/prodrive/herm... PS6, Line 51: a, b, x, y I'd rename these parameters to `section_type, section_name, dest, field_name`
https://review.coreboot.org/c/coreboot/+/45686/6/src/mainboard/prodrive/herm... PS6, Line 66: x, y `dest, field_name` maybe?