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 8:
(6 comments)
https://review.coreboot.org/#/c/19557/4/src/soc/rockchip/rk3399/clock.c File src/soc/rockchip/rk3399/clock.c:
Line 373: /*
0x31f / (2 << 24) == 0.00024% of the intended rate (i.e. 22KHz)? =====> tha
Right, I missed that the fraction is additive (not multiplicative) somehow. You're right then, 0 is the correct value.
https://review.coreboot.org/#/c/19557/8/src/soc/rockchip/rk3399/clock.c File src/soc/rockchip/rk3399/clock.c:
Line 361: * hang in assert() with reboot tests. Which assert() is failing in the failure case? The one below? (That wouldn't really make sense...)
Line 382: (PLL_FRACDIV_MASK << PLL_FRACDIV_SHIFT) & I don't really get what the top part here means. Are you trying to only write the 23:0 bits? Then you'd have to use clrsetbits_le32(), since this is not a normal Rockchip write mask register. But since the rest of the bits are RO anyway, I think it should be fine to just set the whole thing to 0.
PS8, Line 407: 0 Can you not set spreadamp to 8 here already?
PS8, Line 414: Assert This should actually read "Deassert reset", right?
Line 423: divval << PLL_SSMOD_DIVVAL_SHIFT)); You're already writing divval above, why write it again?