Thread (36 messages) flat view 36 messages, 8 authors, 2014-02-19

[PATCH 0/4] clk: mvebu: fix clk init order

From: Gregory CLEMENT <hidden>
Date: 2014-02-17 15:05:06
Also in: lkml

On 17/02/2014 15:42, Emilio L?pez wrote:
Hi Gregory, Ezequiel,

 >> Are we still in time to consider Emilio's oneline proposal?
quoted
quoted
(Emilio: would you mind preparing a suitable patch against dove,
kirkwood, armada370/xp, so we can see the real thing?).
The patch is in a common file, so it does not need patching anything for 
each platform.
quoted
I am still strongly against this proposal because hard-coded the parent
clock name in the driver seems very wrong
It is hardcoded already when the parent is registered, so I do not 
understand your concern.

http://lxr.free-electrons.com/source/drivers/clk/mvebu/common.c?v=3.13#L34
quoted
and moreover in some circumstances
(if there is no output-name, which is our default case) this proposal
just ignored the parent clock given by the device tree and this looked
more wrong.
I have sent a second patch addressing this comment, but you do not seem 
to have taken too serious a look at it.

http://www.spinics.net/lists/arm-kernel/msg305922.html
Please read what I have written: "if there is no output-name, which is our
default case) this proposal just ignored the parent clock given by the device
tree".

Extract of your code from the link you pointed:

const char *default_parent = "tclk";

[...]

of_property_read_string_index(clkspec.np, "clock-output-names",
					      clkspec.args_count ? clkspec.args[0] : 0,
					      &default_parent);

example of a valid dts:
			gateclk: clock-gating-control at 18220 {
				compatible = "marvell,foo-bar-gating-clock";
				reg = <0x18220 0x4>;
				clocks = <&coreclk 1>;
				#clock-cells = <1>;
			};

So in this fictional but still valid example, the device tree indicates that the parent
clock of the gating clock is the 2nd clock provided by the coreclk which is currently
"cpuclk". As no clock-output-names is used, then this will be totally ignore and instead
of using "cpuclk" as parent "tclk" will be used.

I hope this example will show you, what I disagree with this proposal and why it
introduce some regression.

Regards,

Gregory


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help