Ken Lu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38860 )
Change subject: mb/google/hatch: Create palkia variant ......................................................................
Patch Set 17:
(6 comments)
Patch Set 14:
(6 comments)
https://review.coreboot.org/c/coreboot/+/38860/14/src/mainboard/google/hatch... File src/mainboard/google/hatch/variants/palkia/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/38860/14/src/mainboard/google/hatch... PS14, Line 8: PchSerialIoIndexI2C2
Add I2C3 and mark it disabled?
Done
https://review.coreboot.org/c/coreboot/+/38860/14/src/mainboard/google/hatch... PS14, Line 82: 3.2
Should this be in a ACPI_PLD_GROUP with its USB2 pair?
If "type" define to be "UPC_TYPE_INTERNAL" , it should not need to define "ACPI_PLD_GROUP"
https://review.coreboot.org/c/coreboot/+/38860/14/src/mainboard/google/hatch... PS14, Line 110: ELAN Touchscreen
This looks like it is the USI interface, can you add that to the name?
Done
https://review.coreboot.org/c/coreboot/+/38860/14/src/mainboard/google/hatch... PS14, Line 119: stop_gpio
is there any issue having both devices declare the same stop GPIO?
According to our original design , it have 4 control pins for power on/off sequence . 1 for reset , 1 for IRQ , 2 for enable pins . Because we can not define enable_2 or enable2 here . So we pick up stop_gpio to be the 2nd enable pin here .
https://review.coreboot.org/c/coreboot/+/38860/14/src/mainboard/google/hatch... PS14, Line 181: device pci 1e.3 off
missing "end" here
Done
https://review.coreboot.org/c/coreboot/+/38860/14/src/mainboard/google/hatch... PS14, Line 188: HDA
I2S
Done