Skip to contentSkip to conversation

Question about fixed-clock

DISCUSSION Daniel Mack 5 participants 13 messages Feb 18, 2013 – Mar 12, 2013 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

Hi,

This might be a stupid question, but I'm somehow stuck here. I'm using a driver with the following DTS sub-node:

	ref25: ref25M {
		compatible = "fixed-clock";
		#clock-cells = <0>;
		clock-frequency = <25000000>;
	};

	clock-generator at 0 {
				/* ... */
				#clock-cells = <1>;
				clocks = <&ref25>;
	}

The device driver for clock-generator uses something like the following call to get its clock:

	clk = of_clk_get(np, 0);

but the return value is ERR_PTR(-ENOENT) and I also can't find this clock in the clk debugfs tree.

This is on a OMAP/AM33xx device with kernel 3.8-rc7 plus the -next tips of arm-soc and omap, but with no other special clock options selected in the config. Is there anything I'm missing to correctly instantiate the dummy clock?

Thanks for a pointer,
Daniel

#1 lore raw
Quoted 5 lines · Daniel Mack

Hi,

This might be a stupid question, but I'm somehow stuck here. I'm using a driver with the following DTS sub-node:

Quoted 9 lines
        ref25: ref25M {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <25000000>;
        };

        clock-generator at 0 {
                                /* ... */
                                #clock-cells = <1>;

Sorry - this is actually <0>.

#2 lore raw
In reply to Daniel Mack

Hi Daniel,

Quoted 22 lines · Daniel Mack

Hi,

This might be a stupid question, but I'm somehow stuck here. I'm using a driver with the following DTS sub-node:

        ref25: ref25M {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <25000000>;
        };

        clock-generator at 0 {
                                /* ... */
                                #clock-cells = <1>;
                                clocks = <&ref25>;
        }

The device driver for clock-generator uses something like the following call to get its clock:

        clk = of_clk_get(np, 0);
Quoted 7 lines

but the return value is ERR_PTR(-ENOENT) and I also can't find this clock in the clk debugfs tree.

This is on a OMAP/AM33xx device with kernel 3.8-rc7 plus the -next tips of arm-soc and omap, but with no other special clock options selected in the config. Is there anything I'm missing to correctly instantiate the dummy clock?

Have you registered it with clk_register_fixed_rate() ?

In imx we use imx_clk_fixed, which in turns call clk_register_fixed_rate().

Take a look at arch/arm/mach-imx/clk-imx51-imx53.c for a reference.

#3 lore raw

Hi Fabio,
Hi Afzal,

Quoted 31 lines · Fabio Estevam
Quoted 22 lines · Daniel Mack

Hi,

This might be a stupid question, but I'm somehow stuck here. I'm using a driver with the following DTS sub-node:

        ref25: ref25M {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <25000000>;
        };

        clock-generator at 0 {
                                /* ... */
                                #clock-cells = <1>;
                                clocks = <&ref25>;
        }

The device driver for clock-generator uses something like the following call to get its clock:

        clk = of_clk_get(np, 0);
Quoted 7 lines

but the return value is ERR_PTR(-ENOENT) and I also can't find this clock in the clk debugfs tree.

This is on a OMAP/AM33xx device with kernel 3.8-rc7 plus the -next tips of arm-soc and omap, but with no other special clock options selected in the config. Is there anything I'm missing to correctly instantiate the dummy clock?

Quoted 5 lines

Have you registered it with clk_register_fixed_rate() ?

In imx we use imx_clk_fixed, which in turns call clk_register_fixed_rate().

Take a look at arch/arm/mach-imx/clk-imx51-imx53.c for a reference.

Hmm no, I didn't do anything else than adding it to the DT in the first place, hoping that a driver will pick it up and add the clock for me. But it turns out that of_clk_init() is not called at all on my platform.

I'm doing this now from omap_generic_init() and it works. If that's an appropriate place to call it, I can provide a patch.

Thanks,
Daniel

A message that is not in the archive (sent off-list or not yet received): 20130219172246.11471.14635@quantum

#5 lore raw
Quoted 46 lines · Mike Turquette

Quoting Daniel Mack (2013-02-19 01:53:18)

Quoted 45 lines

Hi Fabio,
Hi Afzal,

Quoted 31 lines · Fabio Estevam
Quoted 22 lines · Daniel Mack

Hi,

This might be a stupid question, but I'm somehow stuck here. I'm using a driver with the following DTS sub-node:

        ref25: ref25M {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <25000000>;
        };

        clock-generator at 0 {
                                /* ... */
                                #clock-cells = <1>;
                                clocks = <&ref25>;
        }

The device driver for clock-generator uses something like the following call to get its clock:

        clk = of_clk_get(np, 0);
Quoted 7 lines

but the return value is ERR_PTR(-ENOENT) and I also can't find this clock in the clk debugfs tree.

This is on a OMAP/AM33xx device with kernel 3.8-rc7 plus the -next tips of arm-soc and omap, but with no other special clock options selected in the config. Is there anything I'm missing to correctly instantiate the dummy clock?

Quoted 5 lines

Have you registered it with clk_register_fixed_rate() ?

In imx we use imx_clk_fixed, which in turns call clk_register_fixed_rate().

Take a look at arch/arm/mach-imx/clk-imx51-imx53.c for a reference.

Hmm no, I didn't do anything else than adding it to the DT in the first place, hoping that a driver will pick it up and add the clock for me. But it turns out that of_clk_init() is not called at all on my platform.

Quoted 6 lines
Quoted 3 lines

I'm doing this now from omap_generic_init() and it works. If that's an appropriate place to call it, I can provide a patch.

You can provide a patch, but your example above is for a dummy clock, correct?

No, it's a real clock with a fixed frequency on the board, which feeds another clock chip. I thought this is what fixed-clock is for, just like what a fixed-regulator does? Or how would you describe such a thing in DT, in order to use it as input to another device?

Quoted 2 lines

It would be best to post your patch along with a real-world use for calling of_clk_init().

Not sure what you mean exactly, but the patch is attached. However, I still think I lack some understanding here - the fact that compatible strings have to be passed explicitly from generic code feels wrong.

Thanks,
Daniel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-OMAP-generic-add-call-to-of_clk_init.patch
Type: text/x-patch
Size: 1336 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130219/9fd02682/attachment.bin>

#6 lore raw

Hi eyerone,

Quoted 109 lines · Daniel Mack
Quoted 46 lines · Mike Turquette
Quoted 46 lines

Quoting Daniel Mack (2013-02-19 01:53:18)

Quoted 45 lines
Quoted 45 lines

Hi Fabio,
Hi Afzal,

Quoted 31 lines · Fabio Estevam
Quoted 31 lines · Daniel Mack
Quoted 29 lines
Quoted 29 lines

Hi,

This might be a stupid question, but I'm somehow stuck here. I'm using a driver with the following DTS sub-node:

        ref25: ref25M {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <25000000>;
        };

        clock-generator at 0 {
                                /* ... */
                                #clock-cells = <1>;
                                clocks = <&ref25>;
        }

The device driver for clock-generator uses something like the following call to get its clock:

        clk = of_clk_get(np, 0);

but the return value is ERR_PTR(-ENOENT) and I also can't find this clock in the clk debugfs tree.

This is on a OMAP/AM33xx device with kernel 3.8-rc7 plus the -next tips of arm-soc and omap, but with no other special clock options selected in the config. Is there anything I'm missing to correctly instantiate the dummy clock?

Quoted 5 lines
Quoted 5 lines

Have you registered it with clk_register_fixed_rate() ?

In imx we use imx_clk_fixed, which in turns call clk_register_fixed_rate().

Take a look at arch/arm/mach-imx/clk-imx51-imx53.c for a reference.

Hmm no, I didn't do anything else than adding it to the DT in the first place, hoping that a driver will pick it up and add the clock for me. But it turns out that of_clk_init() is not called at all on my platform.

Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

I'm doing this now from omap_generic_init() and it works. If that's an appropriate place to call it, I can provide a patch.

You can provide a patch, but your example above is for a dummy clock, correct?

No, it's a real clock with a fixed frequency on the board, which feeds another clock chip. I thought this is what fixed-clock is for, just like what a fixed-regulator does? Or how would you describe such a thing in DT, in order to use it as input to another device?

Quoted 2 lines
Quoted 2 lines

It would be best to post your patch along with a real-world use for calling of_clk_init().

Not sure what you mean exactly, but the patch is attached. However, I still think I lack some understanding here - the fact that compatible strings have to be passed explicitly from generic code feels wrong.

Thanks,
Daniel

0001-ARM-OMAP-generic-add-call-to-of_clk_init.patch

From 850120371830ffb5e2146aeb2d21c724d6ded09e Mon Sep 17 00:00:00 2001
From: Daniel Mack <[email protected]>
Date: Tue, 19 Feb 2013 12:05:25 +0100
Subject: [PATCH] ARM: OMAP: generic: add call to of_clk_init()

This is needed to instanciate fixed clocks in the DT.

---
 arch/arm/mach-omap2/board-generic.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/mach-omap2/board-generic.c +6 -0
index 0274ff7..3580f16 100644
@@ -15,6 +15,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/irqdomain.h>
+#include <linux/clk-provider.h>
 
 #include <asm/mach/arch.h>
 
@@ -35,6 +36,11 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
 	{ }
 };
 
+static const __initconst struct of_device_id clk_match[] = {
+	{ .compatible = "fixed-clock", .data = of_fixed_clk_setup, },
+	{}
+};
+
 static void __init omap_generic_init(void)
 {
 	omap_sdrc_init(NULL, NULL);

arch/arm/mach-omap2/board-generic.c static void __init omap_generic_init(void) · line 61

Quoted 7 lines
arch/arm/mach-omap2/board-generic.c +1 -0
@@ -49,6 +55,7 @@ static void __init omap_generic_init(void)
 		omap4_panda_display_init_of();
 	else if (of_machine_is_compatible("ti,omap4-sdp"))
 		omap_4430sdp_display_init_of();
+	of_clk_init(clk_match);
 }
 
 #ifdef CONFIG_SOC_OMAP2420

Any opinion about this approach?

Thanks,
Daniel

#7 lore raw
In reply to Daniel Mack

Hi Daniel,

Quoted 12 lines · Daniel Mack
	ref25: ref25M {
		compatible = "fixed-clock";
		#clock-cells = <0>;
		clock-frequency = <25000000>;
	};

	clock-generator at 0 {
				/* ... */
				#clock-cells = <1>;
				clocks = <&ref25>;
	}
Quoted 7 lines

The device driver for clock-generator uses something like the following call to get its clock:

	clk = of_clk_get(np, 0);

but the return value is ERR_PTR(-ENOENT) and I also can't find this clock in the clk debugfs tree.

Hope invoking of_clk_init before clock generator driver helps

Regards
Afzal

#8 lore raw

Hi Daniel,

Quoted 1 line · Afzal Mohammed

Hope invoking of_clk_init before clock generator driver helps

Mails coming from this id are in my personal capacity.

Quoted 10 lines · Daniel Mack
Quoted 1 line · Fabio Estevam

Have you registered it with clk_register_fixed_rate() ?

Hmm no, I didn't do anything else than adding it to the DT in the first place, hoping that a driver will pick it up and add the clock for me. But it turns out that of_clk_init() is not called at all on my platform.

I'm doing this now from omap_generic_init() and it works. If that's an appropriate place to call it, I can provide a patch.

Initially didn't realize that it was for an am335x based one.

I feel that for a platform having it's clock tree in DT, of_clk_init would take care of it, but if clock tree data is not in DT, clock tree would have to be extended in a non-DT way.

Regards
Afzal

#9 lore raw

Hi Afzal,

thanks for looking into this.

Quoted 15 lines · Afzal Mohammed
Quoted 1 line · Afzal Mohammed

Hope invoking of_clk_init before clock generator driver helps

Mails coming from this id are in my personal capacity.

Quoted 8 lines · Daniel Mack
Quoted 1 line · Fabio Estevam

Have you registered it with clk_register_fixed_rate() ?

Hmm no, I didn't do anything else than adding it to the DT in the first place, hoping that a driver will pick it up and add the clock for me. But it turns out that of_clk_init() is not called at all on my platform.

Quoted 8 lines
Quoted 2 lines

I'm doing this now from omap_generic_init() and it works. If that's an appropriate place to call it, I can provide a patch.

Initially didn't realize that it was for an am335x based one.

I feel that for a platform having it's clock tree in DT, of_clk_init would take care of it, but if clock tree data is not in DT, clock tree would have to be extended in a non-DT way.

Hmm, I don't follow. So for generic OMAP board in general which does *not* have its SoC clocks in DT, the question is who's in charge of registering out-of-SoC fixed clocks that are defined in DT.

Note that the clock I'm dealing with here is _outside_ of the SoC, and I just need to have it in DT, so it can feed another clock chip's input pin.

Grep'ing through arch/arm, it seems that the imx arch does the same thing my patch does, but I could also imagine that it should be done somewhere from the DT core. I copied Grant, Rob and Mark for more comments.

Thanks,
Daniel

#10 lore raw
Quoted 10 lines · Daniel Mack
Quoted 3 lines · Afzal Mohammed

I feel that for a platform having it's clock tree in DT, of_clk_init would take care of it, but if clock tree data is not in DT, clock tree would have to be extended in a non-DT way.

Hmm, I don't follow. So for generic OMAP board in general which does *not* have its SoC clocks in DT, the question is who's in charge of registering out-of-SoC fixed clocks that are defined in DT.

Quoted 6 lines

Note that the clock I'm dealing with here is _outside_ of the SoC, and I just need to have it in DT, so it can feed another clock chip's input pin.

Grep'ing through arch/arm, it seems that the imx arch does the same thing my patch does, but I could also imagine that it should be done somewhere from the DT core. I copied Grant, Rob and Mark for more comments.

Wouldn't this just be set up by the DT in the same way that other off-SoC hardware is?

#11 lore raw
Quoted 14 lines · Mark Brown
Quoted 12 lines · Daniel Mack
Quoted 3 lines · Afzal Mohammed

I feel that for a platform having it's clock tree in DT, of_clk_init would take care of it, but if clock tree data is not in DT, clock tree would have to be extended in a non-DT way.

Hmm, I don't follow. So for generic OMAP board in general which does *not* have its SoC clocks in DT, the question is who's in charge of registering out-of-SoC fixed clocks that are defined in DT.

Note that the clock I'm dealing with here is _outside_ of the SoC, and I just need to have it in DT, so it can feed another clock chip's input pin.

Quoted 6 lines
Quoted 3 lines

Grep'ing through arch/arm, it seems that the imx arch does the same thing my patch does, but I could also imagine that it should be done somewhere from the DT core. I copied Grant, Rob and Mark for more comments.

Wouldn't this just be set up by the DT in the same way that other off-SoC hardware is?

Well, I thought so too. To repeat, in my DT, I have:

	ref25: ref25M {
		compatible = "fixed-clock";
		#clock-cells = <0>;
		clock-frequency = <25000000>;
	};

to represent an osciallator on the board. I need to specify it here so I can pass a reference to another chip:

	si5351a: clock-generator@60 {
		compatible = "silabs,si5351a";
		clocks = <&ref25>;
	};

I would have expected that "fixed-clock" is matched by a driver lurking around for DT boards, just like what the "fixed-regulator" driver does for instance. But the clock device isn't initialized unless board code explicitly calls of_clk_init() with a table mentioning "fixed-clock", as in my patch.

I don't know the clock framework well enough, but it seems that either all DT boards are supposed to do the same in their generic bits (which sounds like a lot of code duplication), or the fixed-clock driver should behave like any other driver wrt its probing from DT. I'm open to suggestions :)

Daniel

#12 lore raw
Quoted 10 lines · Daniel Mack
Quoted 8 lines · Mark Brown
Quoted 4 lines · Daniel Mack

On 07.03.2013 19:42, Afzal Mohammed wrote:
Grep'ing through arch/arm, it seems that the imx arch does the same thing my patch does, but I could also imagine that it should be done somewhere from the DT core. I copied Grant, Rob and Mark for more comments.

Wouldn't this just be set up by the DT in the same way that other off-SoC hardware is?

Quoted 7 lines

[...]

I don't know the clock framework well enough, but it seems that either all DT boards are supposed to do the same in their generic bits (which sounds like a lot of code duplication), or the fixed-clock driver should behave like any other driver wrt its probing from DT. I'm open to suggestions :)

Daniel,

the current common clock framework does not register any of its "core" clocks, i.e. fixed-clock, et.al. I haven't had a look at the way regulator api registers them. But if you find the way reasonable, why not propose a patch for ccf that registers at least a set of the core clocks itself?

Sebastian

#13 lore raw
In reply to Daniel Mack
Quoted 6 lines · Daniel Mack
Quoted 2 lines · Mark Brown

Wouldn't this just be set up by the DT in the same way that other off-SoC hardware is?

Well, I thought so too. To repeat, in my DT, I have:

OK, I got CCed in part way through the thread.

Quoted 11 lines

I would have expected that "fixed-clock" is matched by a driver lurking around for DT boards, just like what the "fixed-regulator" driver does for instance. But the clock device isn't initialized unless board code explicitly calls of_clk_init() with a table mentioning "fixed-clock", as in my patch.

I don't know the clock framework well enough, but it seems that either all DT boards are supposed to do the same in their generic bits (which sounds like a lot of code duplication), or the fixed-clock driver should behave like any other driver wrt its probing from DT. I'm open to suggestions :)

Your expectation sounds like what I'd expect too...