Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39613 )
Change subject: Sc7180: Add display 10nm phy & pll programming support ......................................................................
Patch Set 32:
(7 comments)
This would really go faster if you didn't force me to point out each unused struct member invidiually for you... :/
https://review.coreboot.org/c/coreboot/+/39613/32/src/soc/qualcomm/sc7180/di... File src/soc/qualcomm/sc7180/display/dsi_phy.c:
https://review.coreboot.org/c/coreboot/+/39613/32/src/soc/qualcomm/sc7180/di... PS32, Line 39: static struct dsi_phy_divider_lut_entry_type pll_dividerlut_dphy[] = { If there's an assumption that pll_post_div must always be a power of 2, that should be documented here.
https://review.coreboot.org/c/coreboot/+/39613/32/src/soc/qualcomm/sc7180/di... PS32, Line 79: uint32_t escape_freq; unused?
https://review.coreboot.org/c/coreboot/+/39613/32/src/soc/qualcomm/sc7180/di... PS32, Line 622: for (i = 0; i < 4; i++) { I think you can replace this loop with just log2(pll_cfg->pll_post_div)?
https://review.coreboot.org/c/coreboot/+/39613/32/src/soc/qualcomm/sc7180/di... PS32, Line 739: while (i < ARRAY_SIZE(clks)) { nit: This should really use for, not while.
https://review.coreboot.org/c/coreboot/+/39613/32/src/soc/qualcomm/sc7180/di... File src/soc/qualcomm/sc7180/display/dsi_phy_pll.c:
https://review.coreboot.org/c/coreboot/+/39613/32/src/soc/qualcomm/sc7180/di... PS32, Line 21: u32 pll_lockdet_rate; unused?
https://review.coreboot.org/c/coreboot/+/39613/32/src/soc/qualcomm/sc7180/di... PS32, Line 93: regs->pll_prop_gain_rate = 12; Where is this used?
https://review.coreboot.org/c/coreboot/+/39613/32/src/soc/qualcomm/sc7180/di... PS32, Line 132: 0x08 Should this maybe not be hardcoded?