Subject: [PATCH 4/9] clk: sunxi: PLL2 support for sun4i, sun5i and sun7i

From: [email-masked] (Emilio López)
Date: Sun, 03 Aug 2014 19:02:41 -0300

Hi,

El 03/08/14 a las 09:44, Maxime Ripard escibi?:
> On Thu, Jul 31, 2014 at 06:28:07PM -0300, Emilio L?pez wrote:
>> This patch adds support for PLL2 and derivates on sun4i, sun5i and
>> sun7i SoCs. As this PLL is only used for audio and requires good
>> accuracy, we only support two known good rates.
>>
>> Signed-off-by: Emilio L?pez <[email-masked]>
>> ---
>>
(...)
>> +	/* PLL2x8, double of PLL2 without the post divisor */
>> +	of_property_read_string_index(np, "clock-output-names", 3, &clk_name);
>> +	clks[3] = clk_register_fixed_factor(NULL, clk_name, parent,
>> +					    CLK_SET_RATE_PARENT, 2 * 4, 1);
>
> Why have you declared them here, instead of using fixed factors in the
> DT directly, like we have done in the past?

I'd prefer to see one clock with four outputs represented as one clock 
with four outputs rather than one clock with one output and other three 
weirdly named nodes floating around. I can only see four uses of 
fixed-factor-clock in our DTSI, three of which are "pass through" clocks 
for ahb and ar100 on sun6/8i, and a fourth one to implement the 32k that 
results from reducing the 24M osc on sun7i.

As a second reason, implementing it as fixed factors would be slightly 
incorrect, as it's only "x8" when you use the limited set of factors 
that Allwinner and us are using for audio. If we were ever to support 
the full range of the PLL. the DT binding would suddenly be off. By 
using a single binding, the meaning is self-contained in the driver.

As a third reason, of_fixed_factor_clk_setup() does not set 
CLK_SET_RATE_PARENT, which we'd need to propagate the rate change upstream.

Cheers,

Emilio