On 7/28/2023 11:00 PM, Konrad Dybcio wrote: > On 28.07.2023 15:23, Vikash Garodia wrote: >> From: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx> >> >> This implements ops to initialize, enable and disable extrenal >> resources needed by video driver like power domains, clocks etc. >> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx> >> Signed-off-by: Vikash Garodia <quic_vgarodia@xxxxxxxxxxx> >> --- > There's a whole bunch of kerneldoc abuses (comments should start with > /* and not /**). Make sure you have proper spaces between single-line > C-style comments (e.g. /*Get should be /* Get etc.) > > Capitalizing the first word within the comment would be nice too. Agree! will fix the comments in next version. > > > Do we need a separate bus table? i.e. does it make sense to adjust the > bandwidth values separately from the clock rates? Yes, we do need to vote for bus bandwidth separately hence separate bus table is required. > > Do you think there will be more than one set of msm_vidc_resources_ops? > Perhaps it'd make sense to drop that layer of abstraction if not. Many > function names could drop the __ prefix. > > A whole bunch of d_vpr_h seem almost excessive. Sure, all custom debug wrappers will be removed in next version > > MSM_VIDC_CLKFLAG_* are unused. > Thanks for pointing it, will remove in next version. Thanks, Dikshita > Konrad
