Skip to contentSkip to conversation

clk: ti: Add support for FAPLL on dm816x

DISCUSSION Tony Lindgren 1 participant 1 message Jan 28, 2015 1 message not in the archive

lore
Share & tools
lore
#1 lore raw
In reply to a message not in the archive
Quoted 48 lines · Dan Carpenter

Hello Tony Lindgren,

The patch 163152cbbe32: "clk: ti: Add support for FAPLL on dm816x" from Jan 13, 2015, leads to the following static checker warning:

	drivers/clk/ti/fapll.c:87 ti_fapll_enable()
	warn: double left shift '1 << (1 << (3))'

drivers/clk/ti/fapll.c

    82  static int ti_fapll_enable(struct clk_hw *hw)
    83  {
    84          struct fapll_data *fd = to_fapll(hw);
    85          u32 v = readl_relaxed(fd->base);
    86  
    87          v |= (1 << FAPLL_MAIN_PLLEN);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

FAPLL_MAIN_PLLEN() is BIT(3). It's possible that this code is correct as is, but as a complete outsider I think it's more likely that the code should be:

		v |= FAPLL_MAIN_PLLEN;


    88          writel_relaxed(v, fd->base);
    89  
    90          return 0;
    91  }
    92  
    93  static void ti_fapll_disable(struct clk_hw *hw)
    94  {
    95          struct fapll_data *fd = to_fapll(hw);
    96          u32 v = readl_relaxed(fd->base);
    97  
    98          v &= ~(1 << FAPLL_MAIN_PLLEN);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Same.

    99          writel_relaxed(v, fd->base);
   100  }
   101  
   102  static int ti_fapll_is_enabled(struct clk_hw *hw)
   103  {
   104          struct fapll_data *fd = to_fapll(hw);
   105          u32 v = readl_relaxed(fd->base);
   106  
   107          return v & (1 << FAPLL_MAIN_PLLEN);
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Quoted 3 lines

Same.

   108  }

Thanks for catching that. Yes that's a bug, I've screwed up while cleaning up and means the parent PLL will not get disabled even if all the children are disabled.

Will send a fix shortly.

Regards,

Tony