12 comments:
Patch Set #17, Line 7: sc7180: Add display 10nm phy & pll programming support [Patch 1 of 3]
We do not use the *PATCH 1 of 3* tags in coreboot. Just having them in the same branch, and describing the work in the commit message is enough.
This is too terse for a 2500 lines diffstat. Please elaborate, elaborate on the implementation/data structures, mention the used datasheets and their version, and, if not written from scratch, where the code comes from.
File src/soc/qualcomm/sc7180/display/dsi_phy.c:
The project uses SPDX headers now.
Patch Set #17, Line 30: HAL_DSI_PHY_PLL_READY_TIMEOUT
Please add the unit into the macro name.
Patch Set #17, Line 45: { 2, 11},
Please add a space before the closing }.
Please follow https://doc.coreboot.org/coding_style.html#commenting.
Please use CB_SUCCESS and friends.
Patch Set #17, Line 1049: printk(BIOS_ERR, "%s: pinfo NULL, error\n", __func__);
Error messages should be user understandable. This is written like a debug message. Please elaborate.
Patch Set #17, Line 1053: set up
The noun is spelled without a space. Maybe:
Set up Phy
Patch Set #17, Line 1056: goto out;
Just return?
File src/soc/qualcomm/sc7180/display/dsi_phy_pll.c:
Please remove the blank lines at the end of the file.
File src/soc/qualcomm/sc7180/include/soc/display/display_resources.h:
Please remove the blank lines.
To view, visit change 39613. To unsubscribe, or for help writing mail filters, visit settings.