9 comments:
File src/soc/qualcomm/sc7180/display/dsi_phy.c:
Patch Set #26, Line 92: DSI_LaneID_Max
Done
No, this is not what I meant. The whole constant name needs to be in all caps, so this should be DSI_LANEID_MAX, the one below should be DSI_LANEID_FORCE_32BIT, and all the other constants you have here should follow the same. No identifier in coreboot should ever have mixed case (other than super special exceptions like MiB), they're always either all caps or all small letters.
File src/soc/qualcomm/sc7180/display/dsi_phy.c:
Patch Set #28, Line 95: uint32_t decdiv_start;
There are still plenty of fields (like this one) in here that aren't used in the code at all (anymore). Please check all these structures and remove all unused fields.
Patch Set #28, Line 453: lane_cfg.strength_override = 0;
What do you need these 6 fields for? I don't think you need struct dsi_phy_laneconfig_type at all to be honest. These are all just hardcoded here and never change. Just hardcode them where they're used.
Patch Set #28, Line 495: wmb();
You don't need this wmb() either.
Patch Set #28, Line 650: phy_cfg->dsi_clksel = 1;
Why do you need this field?
Patch Set #28, Line 760: printk(BIOS_INFO, "Desired bitclock:%u\n", (uint32_t)desired_bclk);
Please add proper spacing and a unit, e.g. "Desired bitclock: %uHz"
Patch Set #28, Line 780: phy_cfg.escape_freq = 19;
Why do you need this field?
File src/soc/qualcomm/sc7180/display/dsi_phy_pll.c:
A lot of unused fields in this file too (e.g. all the ssc_ stuff?). Please remove *all* unused fields from all structs!
Patch Set #28, Line 59: static inline u64 div_u64(u64 dividend, u32 divisor)
Why do we have these two functions, anyway? I don't really see what they add. Why write
x = div_u64(a, b);
when you could just write
x = a / b
?
To view, visit change 39613. To unsubscribe, or for help writing mail filters, visit settings.