Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33433 )
Change subject: mainboard/facebook/fbg1701: Configure TC358860 eDP to MIPI controller ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/#/c/33433/4/src/mainboard/facebook/fbg1701/mainb... File src/mainboard/facebook/fbg1701/mainboard.h:
https://review.coreboot.org/#/c/33433/4/src/mainboard/facebook/fbg1701/mainb... PS4, Line 25: } edp_data_t;
There is some dislike about typedeffing structs, specially when they are for one-time use only.
Will change to u8 table in ramstage.c
https://review.coreboot.org/#/c/33433/4/src/mainboard/facebook/fbg1701/mainb... PS4, Line 27: static const edp_data_t mainboard_TC348860_InitTable[] = {
Hmm.. allocation in . […]
No defined-but-unused. Will place table in ramstage.c
https://review.coreboot.org/#/c/33433/4/src/mainboard/facebook/fbg1701/mainb... PS4, Line 167: { 5, 0x00, 0x00, {0x00, 0x00, 0x00, 0x00, 0x00 } },
Done
Done
https://review.coreboot.org/#/c/33433/4/src/mainboard/facebook/fbg1701/mainb... PS4, Line 315: { 5, 0x68, 0x01, {0x54, 0x01, 0x00, 0x00, 0x00 } },
the dummy entry in this array to detect the end of the array is missing here
When wrong with commit.
https://review.coreboot.org/#/c/33433/4/src/mainboard/facebook/fbg1701/mainb... File src/mainboard/facebook/fbg1701/mainboard.c:
https://review.coreboot.org/#/c/33433/4/src/mainboard/facebook/fbg1701/mainb... PS4, Line 25: void mainboard_configure_edp_bridge(void)
Not sure why this was not placed in mainboard/ramstage. […]
Since it's mainboard specific it's placed in the mainboard.c file.