Skip to contentSkip to conversation

Two-output clk provider with standard clk consumer

DISCUSSION Mason 2 participants 4 messages Oct 13, 2015 – Oct 15, 2015 Common clk

lore
Share & tools
lore

Mason

author
#0 lore raw

Hello everyone,

I'm trying to write a clk driver with the following features:
- a single input (crystal oscillator, modeled by fixed-clock)
- two separate outputs (cpuclk and sysclk) derived from two separate internal PLLs (pll0 and pll1)

I would like to use cpuclk as the input to a fixed-factor-clock node (cpuclk/2).

So the device tree would look like this:

	clocks {
		ranges;
		#address-cells = <1>;
		#size-cells = <1>;

		xtal: xtal {
			compatible = "fixed-clock";
			clock-frequency = <27000000>;
			#clock-cells = <0>;
		};

		clkgen: clkgen@10000 {
			compatible = "foo,clkgen";
			reg = <0x10000 0x30>;
			clocks = <&xtal>;
			#clock-cells = <1>;
		};

		periphclk: periphclk {
			compatible = "fixed-factor-clock";
			clocks = <&clkgen 0>;
			clock-mult = <1>;
			clock-div  = <2>;
			#clock-cells = <0>;
		};
	};

Does this look OK so far?

The driver would look like this:
(headers and error handling omitted for brevity)

static struct clk *output[2];
static struct clk_onecell_data clk_data = { output, 2 }; static void __iomem *clkgen_base;

static void __init make_pll(const char *name, const char *parent, void __iomem *reg)

{
	struct clk *clk;
	unsigned int val, mul, div;

	val = readl_relaxed(reg);
	mul = foo(val);
	div = bar(val);
	clk = clk_register_fixed_factor(NULL, name, parent, 0, mul, div);
	printk("clk = %p\n", clk);
}

static void __init clkgen_setup(struct device_node *np)

{
	int ret;
	const char *parent = of_clk_get_parent_name(np, 0);

	clkgen_base = of_iomap(np, 0);
	make_pll("pll0", parent, clkgen_base + 0);
	make_pll("pll1", parent, clkgen_base + 8);
	output[0] = clk_register_divider(NULL, "cpuclk", "pll0", 0,
			clkgen_base + 0x24, 8, 8, CLK_DIVIDER_ONE_BASED, NULL);
	output[1] = clk_register_fixed_factor(NULL, "sysclk", "pll1", 0, 1, 3);

	ret = of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
	printk("ret=%d out0=%p out1=%p\n", ret, output[0], output[1]);
}

CLK_OF_DECLARE(myclkgen, "foo,clkgen", clkgen_setup);

But the periphclk setup fails because the fixed-factor-clock driver only looks up the parent name using of_clk_get_parent_name ("clkgen" in my case) and calls:

clk_register_fixed_factor(NULL, "periphclk", "clkgen", 0, 1, 2);

Which doesn't keep track of the clkgen output index... <confused>

Can someone please point out what I am missing?

Thanks!

author
#1 lore raw

On 13/10/2015 10:59, Mason asked: [noob clk questions]

Hmmm, I'm thinking there might be a better place to ask beginner question. Some subsystems have a dedicated IRC channel. Does clk have one? :-) Or perhaps kernelnewbies? (I'll check the archive.)

Regards.

#2 lore raw

Quoting Mason (2015-10-13 05:06:37)

Quoted 6 lines

On 13/10/2015 10:59, Mason asked: [noob clk questions] =

Hmmm, I'm thinking there might be a better place to ask beginner question. Some subsystems have a dedicated IRC channel. Does clk have one? :-) Or perhaps kernelnewbies? (I'll check the archive.)

Hi Mason,

Please join us in #linux-clk on freenode. And sorry for not responding sooner. I've been traveling on business for more than one month and Stephen has been bearing 100% of the load while I'm away.

Regarding the reliance on string names when clock-output-names is not used, that is an on-going discussion[0] affecting multiple drivers right now. We're trying to find a long-term solution that moves away from string names and uses the DT hierarchy to establish parent-child relationships.

[0] http://lkml.kernel.org/r/<[email protected]>

Regards,
Mike

Quoted 4 lines

=

Regards.
=20

author
#3 lore raw
Quoted 5 lines · Michael Turquette

Regarding the reliance on string names when clock-output-names is not used, that is an on-going discussion[0] affecting multiple drivers right now. We're trying to find a long-term solution that moves away from string names and uses the DT hierarchy to establish parent-child relationships.

Thanks for your help!

The important thing I was missing is that of_clk_get_parent_name() actually looks up the parent's "clock-output-names" property.

  if (of_property_read_string_index(clkspec.np, "clock-output-names", index, &clk_name) < 0)
    clk_name = clkspec.np->name;

but since I had "hardcoded" the clock names, the parent didn't have a "clock-output-names" property.

Regards.