Attention is currently required from: Shelley Chen, 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:
(1 comment)
Patchset:
PS2:
[…]
Yes, it's true that it still needs to compile and boot correctly in between the two patches. But I still think you can do a lot of this work inside the sc7180/clock.c file first without breaking compilation. For example, it looks like you remove the clock_configure_qup() function because it is unused, you're factoring out parts of pll_init_and_set() into a new function (agera_pll_enable()), you turn clock_configure_dfsr() into a thin wrapper for another function (clock_configure_dfsr_table()), and so on. Please make all of these changes inside the sc7180/clock.c file first, without moving anything to a different file, and submit that as a separate patch. That makes it much easier for us to review what actually changed inside the code (because Gerrit will diff it for us and we don't have to follow every change manually). Then in the next patch you can move the relevant parts out of sc7180/clock.c into common/clock.c (and that should be pretty much a straight Cut+Paste at that point without having to make more changes to the contents).
(Also, it looks like this patch is introducing some sc7280-related stuff that sc7180 doesn't need yet, like the zonda_pll_enable() function. Please move that part into CB:50580 instead.