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

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Mar 12, 2019 at 04:57:37PM -0500, 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);

Oh wow, that's almost impossible to now read.

Sometimes you want intermediate variables for the programmer, not for
the compiler.  The end result is the same, but please tell me that you
could figure out what that one single return line actually does :(

Remember, we write C code for people to understand first, and the CPU to
understand second as we have to maintain it for the future.

thanks,

greg k-h