Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36643 )
Change subject: src/mainboard/siemens: Use PTN3460 chip driver ......................................................................
Patch Set 2:
(46 comments)
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl1/lcd_panel.c:
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 22: D use lowercase here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 23: D same here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 30: no tab, just space here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 34: all needed information Here you just read the EDID data. So maybe "EDID data" would be more clear here?
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 36: no tab, just space here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 43: to Either add something here or remove the "to"
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 50: ; * please remove
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 50: pf of
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 60: no tab, just space here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 73: get got
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl4/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 81: I2C slave address I can't see how helpful this is here. Maybe just "PTN4360 DP2LVDS Bridge"?
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl4/lcd_panel.c:
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 22: D lower case
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 23: D same here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 30: no tab, just space here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 34: all needed information EDID data
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 36: no tab, just space here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 43: to remove or add more text
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 50: pf of
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 50: ; * please remove
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 52: to to be
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 60: no tab, just space here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 73: get got
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 87: signal "lanes" would be the better word here.
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl5/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 89: I2C slave address I can't see how helpful this is here. Maybe just "PTN4360 DP2LVDS Bridge"?
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/variants/mc_apl5/lcd_panel.c:
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 45: D lower case
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 46: D same here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 53: space, not tab
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 57: all needed information EDID data
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 61: space, not tab please.
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 68: to add more text or delete
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 75: ; * please remove
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 75: pf of
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 85: space
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_ap... PS2, Line 112: signal lane
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... File src/mainboard/siemens/mc_tcu3/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 56: I2C slave address I can't see how helpful this is here. Maybe just "PTN4360 DP2LVDS Bridge"?
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... File src/mainboard/siemens/mc_tcu3/lcd_panel.c:
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 81: Ane reason for so many spaces here?
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 91: no tab, just space here
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 95: all needed information EDID data
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 97: no tab, just space please
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 104: to either add more text or remove
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 111: ; * plese remove
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 111: pf of
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 113: to to be
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 124: no tab, just space
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 137: get got
https://review.coreboot.org/c/coreboot/+/36643/2/src/mainboard/siemens/mc_tc... PS2, Line 156: signal lane