Re: [Outreachy kernel] [PATCH] Staging: sm750fb: Remove unnecessary local variables

[Madhumthia Prabakaran <madhumithabiw@gmail.com>]

On Tue, Mar 12, 2019 at 11:09:30PM +0100, Julia Lawall wrote:
> 
> 
> On Tue, 12 Mar 2019, Madhumitha Prabakaran wrote:
> 
> > Remove unnecessary local variables in function get_mxclk_freq.
> > Issue found by Coccinelle using ret.cocci.
> >
> > Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> > ---
> >  drivers/staging/sm750fb/ddk750_chip.c | 17 +++++++----------
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
> > index 90f5480304f4..d0462f21fe36 100644
> > --- a/drivers/staging/sm750fb/ddk750_chip.c
> > +++ b/drivers/staging/sm750fb/ddk750_chip.c
> > @@ -33,19 +33,16 @@ void sm750_set_chip_type(unsigned short dev_id, u8 rev_id)
> >
> >  static unsigned int get_mxclk_freq(void)
> >  {
> > -	unsigned int pll_reg;
> > -	unsigned int M, N, OD, POD;
> > -
> >  	if (sm750_get_chip_type() == SM750LE)
> >  		return MHz(130);
> >
> > -	pll_reg = peek32(MXCLK_PLL_CTRL);
> > -	M = (pll_reg & PLL_CTRL_M_MASK) >> PLL_CTRL_M_SHIFT;
> > -	N = (pll_reg & PLL_CTRL_N_MASK) >> PLL_CTRL_N_SHIFT;
> > -	OD = (pll_reg & PLL_CTRL_OD_MASK) >> PLL_CTRL_OD_SHIFT;
> > -	POD = (pll_reg & PLL_CTRL_POD_MASK) >> PLL_CTRL_POD_SHIFT;
> > -
> > -	return DEFAULT_INPUT_CLOCK * M / N / (1 << OD) / (1 << POD);
> > +	return DEFAULT_INPUT_CLOCK * (peek32(MXCLK_PLL_CTRL) &
> > +	       PLL_CTRL_M_MASK) >> PLL_CTRL_M_SHIFT /
> > +	       (peek32(MXCLK_PLL_CTRL) & PLL_CTRL_N_MASK) >>
> > +	       PLL_CTRL_N_SHIFT / (1 << (peek32(MXCLK_PLL_CTRL) &
> > +	       PLL_CTRL_OD_MASK) >> PLL_CTRL_OD_SHIFT) / (1 <<
> > +	       (peek32(MXCLK_PLL_CTRL) & PLL_CTRL_POD_MASK) >>
> > +	       PLL_CTRL_POD_SHIFT);
> 
> I'm not sure what you are doing here. The proposed semantic patch doesn't
> do this.  It only matches cases where there is an assignment to a variable
> and then a return of exactly that variable.
> 
> I have the impression that you are looking at the files that are handled
> by Coccinelle, then searching in those files for something that looks
> relevant, and then making the transformation by hand.  You shouldn't do
> that.  Coccinelle will find the proper places and make the transformation
> for you.  You will have an appropriate patchon standard output. You can
> then apply it to your code, check that it compiles, check that the format
> is OK and checkpatch is happy, etc.
> 
> julia
> 
> 
> 
> >  }
> >
> >  /*
> > --
> > 2.17.1
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190312215737.29200-1-madhumithabiw%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >

Yeah, you are right. I have a misunderstanding about working of Coccinelle. I'm trying to understand it. I will drop this patch.
Thanks,

Madhumitha