clk: Two-output clk provider with standard clk consumer

clk: Two-output clk provider with standard clk consumer

[Mason]

[ This was first sent to the linux-clk at vger ML, but it seems like they
only deal with actual patches, not general noob-ish questions ]

Hello everyone,

I'm trying to write a clk driver. To summarize, the platform has a 27 MHz
crystal feeding two PLLs (pll0 and pll1), each feeding resp. cpuclk and
sysclk. On my first try, I wrote one DT node for each element, and life
was good.

But Stephen Boyd wrote:
> More discussion will come with the binding, but we're pushing
> people towards making real platform drivers for their clock
> controllers, instead of parsing everything out of DT and having
> one node per clock. So if these are picking things out of some
> larger clock controller block, please rewrite the binding to be a
> real clock provider.

So... I've tried 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 at 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!

clk: Two-output clk provider with standard clk consumer

[Mason]

On 13/10/2015 14:30, Mason wrote:

> 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 at 10000 {
> 			compatible = "foo,clkgen";
> 			reg = <0x10000 0x30>;
> 			clocks = <&xtal>;
> 			#clock-cells = <1>;

Missing clock-output-names property here.

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

Sorry for the noise.