[coreboot-gerrit] Change in coreboot[master]: rockchip/rk3399: enable DPLL SSC for DDR EMI test on bob

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/19557 )

Change subject: rockchip/rk3399: enable DPLL SSC for DDR EMI test on bob
......................................................................


Patch Set 4:

(3 comments)

https://review.coreboot.org/#/c/19557/3/src/soc/rockchip/rk3399/clock.c
File src/soc/rockchip/rk3399/clock.c:

Line 633: 	switch (hz) {
> done.
...yeah, I wouldn't know, sorry. You guys are the experts. But we can't land this until we have a clear understanding how/why it works and confidence that it won't break any devices. Some random inconsequential EMI isn't as bad as boot failures.


https://review.coreboot.org/#/c/19557/4/src/soc/rockchip/rk3399/clock.c
File src/soc/rockchip/rk3399/clock.c:

Line 360: 	udelay(30);
Is this delay needed? There's nothing coming before here? If it's still in response to the rkclk_set_pll(dpll), then it should probably go in rkclk_configure_ddr(). (But we're already waiting for PLL_LOCK_STATUS in rkclk_set_pll(), isn't that enough?)


Line 373: 			      PLL_FRAC_MODE << PLL_DSMPD_SHIFT));
So we're enabling fractional mode here, but where are we setting the fractional divider (in DPLL_CON2)? In fact, the TRM says that the power-on reset value is 0x31f... so wouldn't that mean that once you enable this, the PLL will only run with 0x31f / (2 << 24) == 0.00024% of the intended rate (i.e. 22KHz)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I75461d4235bcf55324e6664a1220754e770b4786
Gerrit-PatchSet: 4
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Caesar Wang <wxt at rock-chips.com>
Gerrit-Reviewer: Caesar Wang <wxt at rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Philip Chen <philipchen at google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-HasComments: Yes