27 comments:
Patch Set #26, Line 13: added
Please use present tense: add. (Also below. […]
Done
File src/soc/qualcomm/sc7180/display/dsi_phy.c:
Patch Set #26, Line 92: DSI_LaneID_Max
Please stick to coding style (constants in ALL_CAPS)
Done
Patch Set #26, Line 127: struct dsi_phy_configtype {
Do you need all these structures and do they need all these members? Where's the difference between […]
Done
Patch Set #26, Line 168: void mdss_dsi_phy_reset(void);
Same as in other patches.
Done
Patch Set #26, Line 630: enum cb_err
If it cannot fail, just make it void.
Done
Patch Set #26, Line 679: wmb(); /* ensure dsiclk_sel is always programmed before pll start */
write32() already contains explicit DMBs, you don't need to add explicit barriers.
Done
Patch Set #26, Line 804: uint32_t
pointless cast
Done
What is this supposed to do? fval is already unsigned.
Done
KHz
Done
Patch Set #26, Line 1034: {"disp_cc_mdss_esc0_clk", 0, 1, 0, 0, 0, 0},
Please use […]
Done
Patch Set #26, Line 1055: clk_enable
Just put a literal 1 here, there's no point in this being a variable.
Done
Patch Set #26, Line 1058: fialed
typo
Done
File src/soc/qualcomm/sc7180/display/dsi_phy_pll.c:
Patch Set #23, Line 22: uint32_t
<stdint. […]
Done
File src/soc/qualcomm/sc7180/display/dsi_phy_pll.c:
Patch Set #26, Line 18: #define MHZ_3000 3000000000UL
Please use (3000 * MHz) directly in the code rather than hardcoding constants like this.
Done
Patch Set #26, Line 20: # define do_div(n, base) ( \
Looks like you're not actually using this for what it is, so remove it (you just need a plain _tmp / […]
Done
Patch Set #26, Line 30: #define DIV_ROUND_CLOSEST_ULL(x, divisor)( \
We already have a DIV_ROUND_CLOSEST() (in src/commonlib/include/commonlib/helpers. […]
Done
Patch Set #26, Line 98: struct mdss_pll_resources *rsc
unused?
Done
Patch Set #26, Line 183: 19200000
This is SRC_XO_HZ from <soc/clock. […]
Done
Patch Set #26, Line 223: regs->decimal_div_start, frac);
This should fit on one line.
Done
Patch Set #26, Line 296: 19200000
This too.
Done
Patch Set #26, Line 306: wmb();
Already done by each write32().
Done
File src/soc/qualcomm/sc7180/include/soc/display/display_resources.h:
Patch Set #26, Line 9: MDP_External_Clock_Entry
Please stick to coding style (type names in lower_case).
Done
Patch Set #26, Line 22: uint32_t clk_pll_m;
Looks like we never use MND for the DSI clocks we have, so do we need these fields?
mdss_clock_configure() passes m and n as arguments. instead of sending 0's, it will be readable to have this fields.
File src/soc/qualcomm/sc7180/include/soc/display/display_resources.h:
Patch Set #23, Line 13: uint32_t
<stdint. […]
Done
File src/soc/qualcomm/sc7180/include/soc/display/dsi_phy.h:
Patch Set #23, Line 18: uint32_t
<stdint. […]
Done
File src/soc/qualcomm/sc7180/include/soc/display/dsi_phy_pll.h:
Patch Set #26, Line 16: struct mdss_pll_resources {
This is another one of these structs that looks way larger than necessary. […]
Done
File src/soc/qualcomm/sc7180/include/soc/display/dsi_phy_pll.h:
<stdint. […]
I dont think here we need stdint header here.
To view, visit change 39613. To unsubscribe, or for help writing mail filters, visit settings.