On Wed, Jun 24, 2015 at 01:19:10PM +0300, Imre Deak wrote: > On ke, 2015-06-24 at 15:37 +0530, Jindal, Sonika wrote: > > On 6/18/2015 7:55 PM, Imre Deak wrote: > > > @@ -2427,8 +2431,8 @@ static void bxt_ddi_pll_enable(struct > > > drm_i915_private *dev_priv, > > > temp = I915_READ(BXT_PORT_PLL_EBB_4(port)); > > > temp |= PORT_PLL_RECALIBRATE; > > > I915_WRITE(BXT_PORT_PLL_EBB_4(port), temp); > > > - /* Enable 10 bit clock */ > > I think it is OK to keep this comment here because all the steps are > > mentioned in comments, even "disable 10 bit clock". > > Yea, but some of those comments just say what is really obvious from the > code right afterwards.
Concur with Imre here, comments shouldn't ever explain what the code does, but why. If you need to explain what the code does in comments, then it needs to be refactored (helper function with clear name extracted, better naming of variables/defines, ...). Imo almost all the comments in this code should be remove because they're obvious. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
