Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32405 )
Change subject: kohaku: Update overridetree.cb ......................................................................
Patch Set 1:
(10 comments)
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/kohaku/overridetree.cb:
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 3:
Let's keep this consisten with rest of the entries i.e. use space only.
Done
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 21: PcdSdDetectChk
I don't see this defined for cannonlake. Is that required? […]
Oops, yeah I meant to delete that line.
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 22: sd_acpi_mode
Same with this.
Done
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 28: |
extra space?
Done
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 42: .rise_time_ns = 152, : .fall_time_ns = 30,
These will have to be updated after running tests on the board.
Sure, I just borrowed some numbers from another board w/ the same part for a starting point.
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 73: ACPI_IRQ_WAKE_EDGE_LOW
Wake using same gpio as interrupt will not work until the ITSS configuration is fixed in FSP. […]
Ok, so should I comment out the wake pin for now?
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 90: 09
0x9 to make it consistent with 0x2c above.
Done
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 94: drivers/i2c/da7219
This will require appropriate config to be selected in Kconfig for hatch.
Done
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 109: 1a
0x1a
Done
https://review.coreboot.org/#/c/32405/1/src/mainboard/google/hatch/variants/... PS1, Line 116: # no native SD support : register "sdcard_cd_gpio" = ""
Can you please put this up with rest of the registers?
Done