Attention is currently required from: Ravi kumar, mturney mturney, Julius Werner. Taniya Das 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:
(3 comments)
Patchset:
PS2:
It's very hard to see what you're actually changing in this patch (because it seems that you're remo […]
The first one should make all the actual code changes (including renaming, refactoring, removing, etc.) inside the sc7180 directory...
I am not sure if I understand the intend behind the change in target specific directory first as we would never be able to compile the target code due to unavailability of the common code which would be done in the next patch. At any point of time my understanding is coreboot compilation should not be broken for QCOM while picking up any patch. That was the reason I have the common changes earlier than made the target specific code of SC7180/SC7280 which would indicate the changes made.
https://review.coreboot.org/c/coreboot/+/56589 --> SC7180 https://review.coreboot.org/c/coreboot/+/50580 --> SC7280
To make it clear I have listed the functionality it exposes and have also added the API comments in the code.
As I had to make drivers common across targets I had to refactor a lot of code and made sure that the target specific drivers only use them and does not require to introduce any redundant code. But in case you want more documentation for the common clock APIs I can add those.
File src/soc/qualcomm/common/include/soc/clock_common.h:
https://review.coreboot.org/c/coreboot/+/56588/comment/7ea41e5e_85068c5d 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. […]
We have been using the DIV across the Linux kernel clock drivers and thus kept the similar naming convention. But could update to use QCOM_CLOCK_DIV.
File src/soc/qualcomm/common/include/soc/clock_common.h:
https://review.coreboot.org/c/coreboot/+/56588/comment/b25205c1_cf4285dd PS1, Line 6: define Will update it to use QCOM_CLOCK_DIV and update the corresponding clock drivers to use the same.