Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42899 )
Change subject: Sc7180: Add support for sn65dsi86 bridge ......................................................................
Patch Set 10: Code-Review+2
(9 comments)
LGTM after resolving outstanding comments
https://review.coreboot.org/c/coreboot/+/42899/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42899/3//COMMIT_MSG@7 PS3, Line 7: sc7180: Add support for sn65dsi86 bridge.
Please remove the dot/period at the end of the commit message.
Done
https://review.coreboot.org/c/coreboot/+/42899/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42899/10//COMMIT_MSG@7 PS10, Line 7: Sc7180
All other prefixes use lowercase.
still outstanding
https://review.coreboot.org/c/coreboot/+/42899/3/src/drivers/ti/sn65dsi86bri... File src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c:
https://review.coreboot.org/c/coreboot/+/42899/3/src/drivers/ti/sn65dsi86bri... PS3, Line 97: int
Why? I'm not aware of any rules on signedness either (unless we're doing MISRA now... […]
Assuming resolved.
https://review.coreboot.org/c/coreboot/+/42899/3/src/drivers/ti/sn65dsi86bri... PS3, Line 141: int
unsigned int
Assuming resolved.
https://review.coreboot.org/c/coreboot/+/42899/3/src/drivers/ti/sn65dsi86bri... PS3, Line 224: int
unsigned int?
Assuming resolved.
https://review.coreboot.org/c/coreboot/+/42899/3/src/drivers/ti/sn65dsi86bri... PS3, Line 248: printk(BIOS_ERR, "ERROR: valid dp rate not found");
Display doesn't work, probably? […]
Assuming resolved.
https://review.coreboot.org/c/coreboot/+/42899/3/src/drivers/ti/sn65dsi86bri... PS3, Line 291: printk(BIOS_ERR, "ERROR: PLL lock failure\n");
Display will not work
Assuming resolved.
https://review.coreboot.org/c/coreboot/+/42899/3/src/drivers/ti/sn65dsi86bri... PS3, Line 310: printk(BIOS_ERR, "ERROR: Link training failed");
Yes , Display will not work
Assuming resolved.
https://review.coreboot.org/c/coreboot/+/42899/3/src/drivers/ti/sn65dsi86bri... PS3, Line 337: 360
Then let's wait 400ms instead of 360ms? […]
still outstanding (if the datasheet says 400 we should use 400, right?)