Re: [Linux-fbdev-devel] [PATCH] aty128fb: move block to where it should be in function aty128_var_to_pll

[Jaswinder S. R. <jas...@ke...>]

Hello Ben,

On Mon, 2009-09-21 at 08:19 +1000, Benjamin Herrenschmidt wrote:
> On Sun, 2009-09-20 at 13:44 +0530, Jaswinder Singh Rajput wrote:
> > In function aty128_var_to_pll moving block to where it should be.
> > 
> > This also fixed following compilation warning:
> >   CC [M]  drivers/video/aty/aty128fb.o
> > drivers/video/aty/aty128fb.c: In function ‘aty128_decode_var’:
> > drivers/video/aty/aty128fb.c:1520: warning: ‘pll.post_divider’ may be used uninitialized in this function
> 
> I dislike moving pretty much the whole function 2 indentation levels to
> the right just because gcc is being silly.
> 

Indentation is not an issue if I am not breaking any lines. I am moving
the function where it should be and avoid unnecessary goto(break) and
duplicate checking 'if (i == ARRAY_SIZE(post_dividers))'.

> If you really want to silence that warning, just initialize the value
> 

I am trying to save some CPU cycles and you are suggesting to add some
more ;-)

--
JSR

>  
> > Signed-off-by: Jaswinder Singh Rajput <jas...@gm...>
> > ---
> >  drivers/video/aty/aty128fb.c |   33 ++++++++++++++++-----------------
> >  1 files changed, 16 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/video/aty/aty128fb.c b/drivers/video/aty/aty128fb.c
> > index e4e4d43..fd6746e 100644
> > --- a/drivers/video/aty/aty128fb.c
> > +++ b/drivers/video/aty/aty128fb.c
> > @@ -1328,8 +1328,7 @@ static int aty128_var_to_pll(u32 period_in_ps, struct aty128_pll *pll,
> >  	unsigned char post_dividers[] = {1,2,4,8,3,6,12};
> >  	u32 output_freq;
> >  	u32 vclk;        /* in .01 MHz */
> > -	int i = 0;
> > -	u32 n, d;
> > +	int i;
> >  
> >  	vclk = 100000000 / period_in_ps;	/* convert units to 10 kHz */
> >  
> > @@ -1343,27 +1342,27 @@ static int aty128_var_to_pll(u32 period_in_ps, struct aty128_pll *pll,
> >  	for (i = 0; i < ARRAY_SIZE(post_dividers); i++) {
> >  		output_freq = post_dividers[i] * vclk;
> >  		if (output_freq >= c.ppll_min && output_freq <= c.ppll_max) {
> > +			u32 n, d;
> > +
> >  			pll->post_divider = post_dividers[i];
> > -			break;
> > -		}
> > -	}
> >  
> > -	if (i == ARRAY_SIZE(post_dividers))
> > -		return -EINVAL;
> > +			/* calculate feedback divider */
> > +			n = c.ref_divider * output_freq;
> > +			d = c.ref_clk;
> >  
> > -	/* calculate feedback divider */
> > -	n = c.ref_divider * output_freq;
> > -	d = c.ref_clk;
> > +			pll->feedback_divider = round_div(n, d);
> > +			pll->vclk = vclk;
> >  
> > -	pll->feedback_divider = round_div(n, d);
> > -	pll->vclk = vclk;
> > +			DBG("post %d feedback %d vlck %d output %d "
> > +			    "ref_divider %d vclk_per: %d\n", pll->post_divider,
> > +			    pll->feedback_divider, vclk, output_freq,
> > +			    c.ref_divider, period_in_ps);
> >  
> > -	DBG("post %d feedback %d vlck %d output %d ref_divider %d "
> > -	    "vclk_per: %d\n", pll->post_divider,
> > -	    pll->feedback_divider, vclk, output_freq,
> > -	    c.ref_divider, period_in_ps);
> > +			return 0;
> > +		}
> > +	}
> >  
> > -	return 0;
> > +	return -EINVAL;
> >  }
> >  
> >