Change in coreboot[master]: soc/qualcomm/common: Make clock_configure() check for exact matches
Attention is currently required from: Paul Menzel, Julius Werner. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63289 )
Change subject: soc/qualcomm/common: Make clock_configure() check for exact matches ......................................................................
Patch Set 6:
(7 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63289/comment/94fecc01_9a38bda4 PS5, Line 7: Add strict flag to clock_configure()
You do not only add the flag, you also change behavior. […]
We ended up removing the flag, but changing the behavior to check for exact matches all the time. The commit message has been updated to explain the reason for the change and what the behavior change is.
https://review.coreboot.org/c/coreboot/+/63289/comment/8cebb4c9_e4f63e62 PS5, Line 12:
What call sites do you change, and why? Please describe the problem.
Every device that calls clock_configure() (SPI, emmc, sd card, edp, etc.) will be effected by this change.
File src/soc/qualcomm/common/clock.c:
https://review.coreboot.org/c/coreboot/+/63289/comment/558fc7b8_b60cf416 PS5, Line 104: assert(hz == clk_cfg[idx].hz)
Why assert and not log an error?
This has been changed to a die() in the latest patchset.
File src/soc/qualcomm/common/include/soc/clock_common.h:
https://review.coreboot.org/c/coreboot/+/63289/comment/9671d103_850bfc70 PS5, Line 151: strict=1
strict=true
This does not apply anymore.
https://review.coreboot.org/c/coreboot/+/63289/comment/42bb109d_90f2c7ab PS5, Line 158: strict
Maybe exact_match?
This does not apply anymore in the latest patchset.
File src/soc/qualcomm/sc7180/clock.c:
https://review.coreboot.org/c/coreboot/+/63289/comment/1ab2c715_f3a198f8 PS4, Line 218: &mdss_clk_cfg, 0, 0, false);
Sure that works too, just make sure it die()s or something when running over the end of the array.
Done
File src/soc/qualcomm/sc7180/clock.c:
https://review.coreboot.org/c/coreboot/+/63289/comment/23c45f63_d6e6c47f PS5, Line 120: clock_configure
Instead of having one more argument in the signature, I’d prefer a new function […]
This does not apply anymore in the latest patchset. We got rid of the strict parameter so there's only one version of the function.
-
Shelley Chen (Code Review)