Attention is currently required from: Ravi kumar, Taniya Das, mturney mturney. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56588 )
Change subject: soc: common: clock: Add support for common clock driver ......................................................................
Patch Set 2:
(2 comments)
Patchset:
PS2: It's very hard to see what you're actually changing in this patch (because it seems that you're removing some unused stuff and renaming/refactoring some other parts). Can you please split this up into two patches? The first one should make all the actual code changes (including renaming, refactoring, removing, etc.) inside the sc7180 directory, and the second one should just move code from sc7180 to common without changing the contents (as far as possible -- if there's some functional change you can't pull into a separate patch, please point it out explicitly in the commit message).
File src/soc/qualcomm/common/include/soc/clock_common.h:
https://review.coreboot.org/c/coreboot/+/56588/comment/82fbcb44_e7451aaa PS2, Line 6: #define DIV(div) (2 * div - 1) If you're gonna expose this in a header file, please give it a better-namespaced name (e.g. at least CLOCK_DIV() or QCOM_CLOCK_DIV() or something like that), to avoid possible naming clashes in everything that includes this. Or, if you want to keep it short but share it between the different SoC clock.c files, put it in a private header (e.g. <soc/clock_private.h>) that is only included from the clock.c files, but not from the clock.h header that other files calling the clock API need to include.
-
Julius Werner (Code Review)