* re: clk: samsung: add infrastructure to register cpu clocks
@ 2015-06-26 9:54 Dan Carpenter
2015-06-26 10:03 ` Krzysztof Kozlowski
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2015-06-26 9:54 UTC (permalink / raw)
To: thomas.ab; +Cc: linux-samsung-soc, linux-clk
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 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: clk: samsung: add infrastructure to register cpu clocks
2015-06-26 9:54 clk: samsung: add infrastructure to register cpu clocks Dan Carpenter
@ 2015-06-26 10:03 ` Krzysztof Kozlowski
2015-06-26 10:20 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2015-06-26 10:03 UTC (permalink / raw)
To: Dan Carpenter, Bartlomiej Zolnierkiewicz
Cc: thomas.ab, linux-samsung-soc, linux-clk
2015-06-26 18:54 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> 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 }
+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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: clk: samsung: add infrastructure to register cpu clocks
2015-06-26 10:03 ` Krzysztof Kozlowski
@ 2015-06-26 10:20 ` Bartlomiej Zolnierkiewicz
2015-06-26 10:47 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2015-06-26 10:20 UTC (permalink / raw)
To: Krzysztof Kozlowski, Dan Carpenter
Cc: thomas.ab, linux-samsung-soc, linux-clk
Hi,
On Friday, June 26, 2015 07:03:25 PM Krzysztof Kozlowski wrote:
> 2015-06-26 18:54 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> > 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.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> > 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: clk: samsung: add infrastructure to register cpu clocks
2015-06-26 10:20 ` Bartlomiej Zolnierkiewicz
@ 2015-06-26 10:47 ` Dan Carpenter
2015-06-26 11:06 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2015-06-26 10:47 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Krzysztof Kozlowski, thomas.ab, linux-samsung-soc, linux-clk
On Fri, Jun 26, 2015 at 12:20:35PM +0200, Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> On Friday, June 26, 2015 07:03:25 PM Krzysztof Kozlowski wrote:
> > 2015-06-26 18:54 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> > > 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.
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.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: clk: samsung: add infrastructure to register cpu clocks
2015-06-26 10:47 ` Dan Carpenter
@ 2015-06-26 11:06 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 5+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2015-06-26 11:06 UTC (permalink / raw)
To: Dan Carpenter
Cc: Krzysztof Kozlowski, thomas.ab, linux-samsung-soc, linux-clk
On Friday, June 26, 2015 01:47:32 PM Dan Carpenter wrote:
> On Fri, Jun 26, 2015 at 12:20:35PM +0200, Bartlomiej Zolnierkiewicz wrote:
> >
> > Hi,
> >
> > On Friday, June 26, 2015 07:03:25 PM Krzysztof Kozlowski wrote:
> > > 2015-06-26 18:54 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>:
> > > > 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.
>
> 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.
Ah, indeed. It works because we are always also setting BIT(1) in
exynos4_clk_init().
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-26 11:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-26 9:54 clk: samsung: add infrastructure to register cpu clocks Dan Carpenter
2015-06-26 10:03 ` Krzysztof Kozlowski
2015-06-26 10:20 ` Bartlomiej Zolnierkiewicz
2015-06-26 10:47 ` Dan Carpenter
2015-06-26 11:06 ` Bartlomiej Zolnierkiewicz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.