Julius Werner has posted comments on this change. ( https://review.coreboot.org/19477 )
Change subject: rockchip/rk3399: Add MIPI driver ......................................................................
Patch Set 5:
(3 comments)
Thanks. Looks good to me now, essentially, but I'd still like one type change to be sure that we can't have any overflows.
https://review.coreboot.org/#/c/19477/5/src/soc/rockchip/rk3399/include/soc/... File src/soc/rockchip/rk3399/include/soc/mipi.h:
Line 280: unsigned int lane_bps; /* per lane */ Better make this u64 to reduce the risk of transient overflows when we do math on it.
https://review.coreboot.org/#/c/19477/5/src/soc/rockchip/rk3399/mipi.c File src/soc/rockchip/rk3399/mipi.c:
Line 285: lbcc = hcomponent * dsi->lane_bps / (8 * MSECS_PER_SEC); This one makes me a bit uncomfortable. Changing lane_bps to u64 should guarantee that it's safe.
Line 432: mdelay(120); You reduced the delay but you still didn't explain what it's for. Can you please do that (ideally add a comment for that)?
Are all these delays only specific to the SoC or are any of them also dependent on the panel?