Skip to contentSkip to conversation

ARM: smp_twd: Fix twd_get_clock() to get clock from dts or clock framework

RFC PATCH New dinguyen at altera.com 2 participants 3 messages Aug 9, 2013 – Aug 21, 2013 Arm (32-bit)

lore
Share & tools
lore

dinguyen at altera.com

author
#0 lore raw

RFC PATCH 1/1 ARM: smp_twd: Fix twd_get_clock() to get clock from dts or clock framework

From: Dinh Nguyen <[email protected]>

Some platforms have a clock for the smp_twd that goes through a fixed divider. Registering this smp_twd clocks is done by calling clk_register_fixed_factor().

Fix up twd_get_clock() so that it can get the clock from the device tree node or clk_register_fixed_factor().

Signed-off-by Dinh Nguyen <[email protected]>
Cc Viresh Kumar <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Rob Herring <[email protected]>
Cc Arnd Bergmann <[email protected]>
CC Russell King <[email protected]>
Cc spear-devel at list.st.com
Cc linux-arm-kernel at lists.infradead.org
---
 arch/arm/kernel/smp_twd.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
arch/arm/kernel/smp_twd.c +4 -1
index 2595620..643b226 100644
@@ -244,7 +244,10 @@ static void twd_get_clock(struct device_node *np)
 
 	if (np)
 		twd_clk = of_clk_get(np, 0);
-	else
+
+	/* Some platforms do not register the smp_twd clock in the device */
+	/* node of the twd timer. */
+	if (IS_ERR(twd_clk))	
 		twd_clk = clk_get_sys("smp_twd", NULL);
 
 	if (IS_ERR(twd_clk)) {
Signature · 1 line
1.7.9.5
NAK
#1 lore raw

RFC PATCH 1/1 ARM: smp_twd: Fix twd_get_clock() to get clock from dts or clock framework

Quoted 10 lines · On Sat, Aug 10, 2013 at 12:35 AM, <[email protected]> wrote:

From: Dinh Nguyen <[email protected]>

Some platforms have a clock for the smp_twd that goes through a fixed divider. Registering this smp_twd clocks is done by calling clk_register_fixed_factor().

Fix up twd_get_clock() so that it can get the clock from the device tree node or clk_register_fixed_factor().

Signed-off-by Dinh Nguyen <[email protected]>

(...)

arch/arm/kernel/smp_twd.c static void twd_get_clock(struct device_node *np) · line 251

Quoted 8 lines
        if (np)
                twd_clk = of_clk_get(np, 0);
arch/arm/kernel/smp_twd.c +4 -1
-       else
+
+       /* Some platforms do not register the smp_twd clock in the device */
+       /* node of the twd timer. */
+       if (IS_ERR(twd_clk))
                twd_clk = clk_get_sys("smp_twd", NULL);

NACK.

The patch commit message and the patch itself does not match. This has nothing to do with using fixed factor or whatever.

What it wants to do is use clk_get_sys() in case of_clk_get() fails, so DT clocks take precedence.

But this patch is buggy. If you have only platform data, and boot without device tree, np will be NULL, and twd_clk will also be NULL and thus clk_get_sys() will not execute and the system is bugged up.

Yours,
Linus Walleij

author
#2 lore raw

RFC PATCH 1/1 ARM: smp_twd: Fix twd_get_clock() to get clock from dts or clock framework

Quoted 27 lines · Linus Walleij
Quoted 10 lines · On Sat, Aug 10, 2013 at 12:35 AM, <[email protected]> wrote:

From: Dinh Nguyen <[email protected]>

Some platforms have a clock for the smp_twd that goes through a fixed divider. Registering this smp_twd clocks is done by calling clk_register_fixed_factor().

Fix up twd_get_clock() so that it can get the clock from the device tree node or clk_register_fixed_factor().

Signed-off-by Dinh Nguyen <[email protected]>

(...)

arch/arm/kernel/smp_twd.c static void twd_get_clock(struct device_node *np) · line 251

Quoted 8 lines
        if (np)
                twd_clk = of_clk_get(np, 0);
arch/arm/kernel/smp_twd.c +4 -1
-       else
+
+       /* Some platforms do not register the smp_twd clock in the device */
+       /* node of the twd timer. */
+       if (IS_ERR(twd_clk))
                twd_clk = clk_get_sys("smp_twd", NULL);

NACK.

The patch commit message and the patch itself does not match. This has nothing to do with using fixed factor or whatever.

Quoted 7 lines

What it wants to do is use clk_get_sys() in case of_clk_get() fails, so DT clocks take precedence.

But this patch is buggy. If you have only platform data, and boot without device tree, np will be NULL, and twd_clk will also be NULL and thus clk_get_sys() will not execute and the system is bugged up.

Ah, I apologize. I did not even think about the none device tree case.

Dinh

Quoted 4 lines

Yours,
Linus Walleij