clk: samsung: add infrastructure to register cpu clocks

Hello Thomas Abraham,

The patch ddeac8d968d4: "clk: samsung: add infrastructure to register cpu clocks" from Apr 3, 2015, leads to the following static checker warning:

drivers/clk/samsung/clk-cpu.c:164 exynos_cpuclk_pre_rate_change()
warn: test_bit() takes a bit number

drivers/clk/samsung/clk-cpu.c
158 /*

   159           * For the selected PLL clock frequency, get the pre-defined divider
   160           * values. If the clock for sclk_hpm is not sourced from apll, then
   161           * the values for DIV_COPY and DIV_HPM dividers need not be set.
   162           */
   163          div0 = cfg_data->div0;
   164          if (test_bit(CLK_CPU_HAS_DIV1, &cpuclk->flags)) {
                             ^^^^^^^^^^^^^^^^

CLK_CPU_HAS_DIV1 is (1 << 0). We sometimes used it correctly as a mask and sometimes incorrectly (like here) as a bit number.

   165                  div1 = cfg_data->div1;
   166                  if (readl(base + E4210_SRC_CPU) & E4210_MUX_HPM_MASK)
   167                          div1 = readl(base + E4210_DIV_CPU1) &
   168                                  (E4210_DIV1_HPM_MASK | E4210_DIV1_COPY_MASK);
   169          }

2015-06-26 18:54 GMT+09:00 Dan Carpenter <[email protected]>:

Quoted 20 lines

Hello Thomas Abraham,

The patch ddeac8d968d4: "clk: samsung: add infrastructure to register cpu clocks" from Apr 3, 2015, leads to the following static checker warning:

drivers/clk/samsung/clk-cpu.c:164 exynos_cpuclk_pre_rate_change()
warn: test_bit() takes a bit number

drivers/clk/samsung/clk-cpu.c
158 /*

   159           * For the selected PLL clock frequency, get the pre-defined divider
   160           * values. If the clock for sclk_hpm is not sourced from apll, then
   161           * the values for DIV_COPY and DIV_HPM dividers need not be set.
   162           */
   163          div0 = cfg_data->div0;
   164          if (test_bit(CLK_CPU_HAS_DIV1, &cpuclk->flags)) {
                             ^^^^^^^^^^^^^^^^

CLK_CPU_HAS_DIV1 is (1 << 0). We sometimes used it correctly as a mask and sometimes incorrectly (like here) as a bit number.

Quoted 5 lines

   165                  div1 = cfg_data->div1;
   166                  if (readl(base + E4210_SRC_CPU) & E4210_MUX_HPM_MASK)
   167                          div1 = readl(base + E4210_DIV_CPU1) &
   168                                  (E4210_DIV1_HPM_MASK | E4210_DIV1_COPY_MASK);
   169          }

+Cc Bartlomiej

Although original author is Thomas but he abandoned this patch and this was actually Bartlomiej's work mostly. Bart, care to look at it?

Best regards,
Krzysztof

Hi,

Quoted 21 lines · Krzysztof Kozlowski

2015-06-26 18:54 GMT+09:00 Dan Carpenter <[email protected]>:

Quoted 20 lines

Hello Thomas Abraham,

The patch ddeac8d968d4: "clk: samsung: add infrastructure to register cpu clocks" from Apr 3, 2015, leads to the following static checker warning:

drivers/clk/samsung/clk-cpu.c:164 exynos_cpuclk_pre_rate_change()
warn: test_bit() takes a bit number

drivers/clk/samsung/clk-cpu.c
158 /*

   159           * For the selected PLL clock frequency, get the pre-defined divider
   160           * values. If the clock for sclk_hpm is not sourced from apll, then
   161           * the values for DIV_COPY and DIV_HPM dividers need not be set.
   162           */
   163          div0 = cfg_data->div0;
   164          if (test_bit(CLK_CPU_HAS_DIV1, &cpuclk->flags)) {
                             ^^^^^^^^^^^^^^^^

CLK_CPU_HAS_DIV1 is (1 << 0). We sometimes used it correctly as a mask and sometimes incorrectly (like here) as a bit number.

Fortunately the current value of this flag is "1" so test_bit() still works correctly. I'll fix it up later anyway. Thanks for catching it.

Quoted 26 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 21 lines · Krzysztof Kozlowski

2015-06-26 18:54 GMT+09:00 Dan Carpenter <[email protected]>:

Quoted 20 lines

Hello Thomas Abraham,

The patch ddeac8d968d4: "clk: samsung: add infrastructure to register cpu clocks" from Apr 3, 2015, leads to the following static checker warning:

drivers/clk/samsung/clk-cpu.c:164 exynos_cpuclk_pre_rate_change()
warn: test_bit() takes a bit number

drivers/clk/samsung/clk-cpu.c
158 /*

   159           * For the selected PLL clock frequency, get the pre-defined divider
   160           * values. If the clock for sclk_hpm is not sourced from apll, then
   161           * the values for DIV_COPY and DIV_HPM dividers need not be set.
   162           */
   163          div0 = cfg_data->div0;
   164          if (test_bit(CLK_CPU_HAS_DIV1, &cpuclk->flags)) {
                             ^^^^^^^^^^^^^^^^

CLK_CPU_HAS_DIV1 is (1 << 0). We sometimes used it correctly as a mask and sometimes incorrectly (like here) as a bit number.

Quoted 2 lines

Fortunately the current value of this flag is "1" so test_bit() still works correctly. I'll fix it up later anyway. Thanks for catching it.

I don't think it works, although I may have misread. We set BIT(0) in exynos4_clk_init() but we test BIT(1) here.