[coreboot-gerrit] Change in coreboot[master]: rockchip/rk3399: Add MIPI driver

[Anonymous Coward (Code Review)]

nickey.yang at rock-chips.com has posted comments on this change. ( https://review.coreboot.org/19477 )

Change subject: rockchip/rk3399: Add MIPI driver
......................................................................


Patch Set 4:

(7 comments)

https://review.coreboot.org/#/c/19477/2/src/soc/rockchip/rk3399/mipi.c
File src/soc/rockchip/rk3399/mipi.c:

Line 68: 			return dptdin_map[i].testdin;
> Okay, fair enough. But shouldn't the comparison be (dptdin_map[i].max_mbps 
if max_mbps = 100； when we use >= will return 0x10,but we want 0x20


Line 192: 	pclk = edid->mode.pixel_clock * MSEC_PER_SEC;
> That is not a good reason to have bad code, though. You're doing a lot of i
Done


Line 406: 
> Does this need to be part of the structure? Do you have more than one MIPI 
rk3288 and rk3399 have the same mipi ip，it will be more convenient if we want to support 3288 in the future


https://review.coreboot.org/#/c/19477/3/src/soc/rockchip/rk3399/mipi.c
File src/soc/rockchip/rk3399/mipi.c:

PS3, Line 193: big
> *bigger
Done


PS3, Line 194: i->lanes * bpp / 8
> This is also a bad integer division in the 18bpp case, btw. You should prob
Done


PS3, Line 429: 	ret = rk_mipi_dsi_dcs_transfer(&rk_mipi, MIPI_DCS_EXIT_SLEEP_
> Check return value?
Done


PS3, Line 431: 		return;
> here too
Done


-- 
To view, visit https://review.coreboot.org/19477
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I02475eefb187c619c614b1cd20e97074bc8d917f
Gerrit-PatchSet: 4
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: nickey.yang at rock-chips.com
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Sean Paul <seanpaul at chromium.org>
Gerrit-Reviewer: Shunqian Zheng <zhengsq at rock-chips.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Reviewer: nickey.yang at rock-chips.com
Gerrit-HasComments: Yes