Re: [PATCH 2/5] drm/i915/bxt: add missing DDI PLL registers to the state checking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux