Re: clk: iproc: add initial common clock support

[Ray Jui <rjui@broadcom.com>]

Hi Dan,

On 6/26/2015 3:23 AM, Dan Carpenter wrote:
> Hello Ray Jui,
> 
> The patch 5fe225c105fd: "clk: iproc: add initial common clock
> support" from May 5, 2015, leads to the following static checker
> warning:
> 
> 	drivers/clk/bcm/clk-iproc-pll.c:369 iproc_pll_recalc_rate()
> 	warn: should 'ndiv_int << ctrl->ndiv_int.shift' be a 64 bit type?
> 
> drivers/clk/bcm/clk-iproc-pll.c
>   341  static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
>    342                                             unsigned long parent_rate)
>    343  {
>    344          struct iproc_clk *clk = to_iproc_clk(hw);
>    345          struct iproc_pll *pll = clk->pll;
>    346          const struct iproc_pll_ctrl *ctrl = pll->ctrl;
>    347          u32 val;
>    348          u64 ndiv;
>    349          unsigned int ndiv_int, ndiv_frac, pdiv;
>    350  
>    351          if (parent_rate == 0)
>    352                  return 0;
>    353  
>    354          /* PLL needs to be locked */
>    355          val = readl(pll->pll_base + ctrl->status.offset);
>    356          if ((val & (1 << ctrl->status.shift)) == 0) {
>    357                  clk->rate = 0;
>    358                  return 0;
>    359          }
>    360  
>    361          /*
>    362           * PLL output frequency =
>    363           *
>    364           * ((ndiv_int + ndiv_frac / 2^20) * (parent clock rate / pdiv)
>    365           */
>    366          val = readl(pll->pll_base + ctrl->ndiv_int.offset);
>    367          ndiv_int = (val >> ctrl->ndiv_int.shift) &
>    368                  bit_mask(ctrl->ndiv_int.width);
>    369          ndiv = ndiv_int << ctrl->ndiv_int.shift;
> 
> ndiv is declared u64 but only the lower 32 bits are used because of
> shift wrapping.
> 
>    370  
>    371          if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
>    372                  val = readl(pll->pll_base + ctrl->ndiv_frac.offset);
>    373                  ndiv_frac = (val >> ctrl->ndiv_frac.shift) &
>    374                          bit_mask(ctrl->ndiv_frac.width);
>    375  
>    376                  if (ndiv_frac != 0)
>    377                          ndiv = (ndiv_int << ctrl->ndiv_int.shift) | ndiv_frac;
>                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Here as well.
> 
>    378          }
> 
> regards,
> dan carpenter
> 

Got it. I'll fix both.

Thanks,

Ray