Attention is currently required from: Shelley Chen, Ravi kumar, Taniya Das, mturney mturney, Sudheer Amrabadi. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56587 )
Change subject: qualcomm/sc7180: Cleanups for drivers with common clock ......................................................................
Patch Set 5:
(6 comments)
File src/soc/qualcomm/sc7180/clock.c:
https://review.coreboot.org/c/coreboot/+/56587/comment/5334a964_770664cd PS5, Line 131: int When using the CB_xxx error values, please use the return type `enum cb_err` instead of a standard integer type (applies to multiple functions in here).
https://review.coreboot.org/c/coreboot/+/56587/comment/7a072c5f_f52154b9 PS5, Line 172: while This should be an `if` now
https://review.coreboot.org/c/coreboot/+/56587/comment/b07c81d4_c6464011 PS5, Line 183: while same
https://review.coreboot.org/c/coreboot/+/56587/comment/a2ded891_7d77478c PS5, Line 311: if (ret < 0) If you want to use the cb_err type, use it consistently (i.e. this should check `if (ret != CB_SUCCESS)` or something like that. Never treat cb_err as an integer or mix it with integers in the same variable.
https://review.coreboot.org/c/coreboot/+/56587/comment/81b36a5b_9a2ff87a PS5, Line 356: divider nit: Would be easier to just say `divider * 2 - 1` here?
File src/soc/qualcomm/sc7180/watchdog.c:
https://review.coreboot.org/c/coreboot/+/56587/comment/6eef91de_c68b07e4 PS5, Line 8: #define WDOG_RESET_BIT_MASK 1 If you add this here you also should remove it from clock.h in this same patch.