[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 9:

(2 comments)

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

Line 361: 	 * hang somewhere with reboot tests.
> Yes, it's related to "assert(dpll_cfg->refdiv && dpll_cfg->refdiv <= 6);"
But that makes absolutely no sense. dpll_cfg->refdiv is hardcoded in rkclk_configure_ddr(). It can only ever be 1 or 2 with the current code. There is no way a delay could affect that in any way.

Please post the log of the assertion error you see without this. Are you *sure* that the assertion below is actually the one that you trigger?

(Note that if you just see it hang with no further output, that's not an assertion error. Assertions will output an "ASSERTION ERROR: file '...', line ..." line to the console. If you're seeing a hang, it must be from some of the register writes below... you could try moving the udelay further down line by line to figure out which one.)


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

Line 424: 			      8 << PLL_SSMOD_SPREADAMP_SHIFT));
Now you're already setting spreadamp to 8 above, you can get rid of this.


-- 
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: 9
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