Thread (1 message) 1 message, 1 author, 2016-01-21

Re: [RFC 1/4] WIP: MIPS: ath79: make ar933x clks more devicetree-friendly

From: Alban <hidden>
Date: 2016-01-21 11:03:20
Also in: linux-mips

On Thu, 21 Jan 2016 13:17:11 +0300
Antony Pavlov [off-list ref] wrote:
On Thu, 21 Jan 2016 09:12:17 +0100
Alban [off-list ref] wrote:
quoted
On Thu, 21 Jan 2016 03:12:15 +0300
Antony Pavlov [off-list ref] wrote:
quoted
On Mon, 18 Jan 2016 20:57:25 +0100
Alban [off-list ref] wrote:
quoted
On Mon, 18 Jan 2016 02:56:24 +0300
Antony Pavlov [off-list ref] wrote:
quoted
At the moment ar933x of-enabled drivers use use clock names
(e.g. "uart" or "ahb") to get clk descriptor.
On the other hand
Documentation/devicetree/bindings/clock/clock-bindings.txt states
that the 'clocks' property is required for passing clk to clock
consumers.
This patch is not need, you should set the clock-names property in
the relevant device nodes instead.
This patch is needed for AR9331!

In ar933x_clocks_init() we have

        ath79_add_sys_clkdev("ref", ref_rate);
        clks[0] = ath79_add_sys_clkdev("cpu", cpu_rate);
        clks[1] = ath79_add_sys_clkdev("ddr", ddr_rate);
        clks[2] = ath79_add_sys_clkdev("ahb", ahb_rate);

        clk_add_alias("wdt", NULL, "ahb", NULL);
        clk_add_alias("uart", NULL, "ref", NULL);

"uart" is an alias for "ref". But "ref" is not visible via device tree!

I see this error message on ar933x-uart start:
 
     ERROR: could not get clock /ahb/apb/uart@18020000:uart(0)
The ref clock should be defined in the board DTS, I now see that it is
missing in yours. What you need to do is to define the clock-names
property in the Soc DTS, that allow the names lookup to work. Then in
the board DTS you can define the clock property to connect it to the
proper parent. 

I'm also working on supporting the QCA9558 and the clock tree is similar.
See https://github.com/AlbanBedel/linux/commit/d6c8f8adfce08972c6
as example.
Current ath79 clock.c code does not read ref clock from devicetree!
So you can set any clock rate value in board DTS but it will has no effect
on the real clk calculation.
Yes, the ath79 clock is broken in this respect, however the uart driver
isn't and it will use the clock it receive. The fixed oscillator works
fine on QCA9558 which also use the ref clock for the uart.
A more reasonable solution is used for CI20 board.
In arch/mips/boot/dts/ingenic/jz4780.dtsi we have

	ext: ext {
		compatible = "fixed-clock";
		#clock-cells = <0>;
	};

...

	cgu: jz4780-cgu@10000000 {
		compatible = "ingenic,jz4780-cgu";
		reg = <0x10000000 0x100>;

		clocks = <&ext>, <&rtc>;
		clock-names = "ext", "rtc";

		#clock-cells = <1>;
	};


In arch/mips/boot/dts/ingenic/ci20.dts we have

&ext {
	clock-frequency = <48000000>;
};

At last drivers/clk/ingenic/jz4780-cgu.c registers this "ext" clock
as a parent of most other subordianate clocks. So there is no magic
frequency constants in drivers/clk/ingenic!

In arch/mips/ath79/clocks.c we have a very different situation:
the reference clock frequences are already hardcoded in C-code so there is
no need to mention them in devicetree files.
No, we need to fix the code to only use the hard coded values for the
legacy platforms.

Alban
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help