linux-kernel - Re: [PATCH 0/4] clk: mvebu: fix clk init order

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53022B59.3060905@elopez.com.ar>
Date:	Mon, 17 Feb 2014 12:31:37 -0300
From:	Emilio López <emilio@...pez.com.ar>
To:	Gregory CLEMENT <gregory.clement@...e-electrons.com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
CC:	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	Jason Cooper <jason@...edaemon.net>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Andrew Lunn <andrew@...n.ch>,
	Mike Turquette <mturquette@...aro.org>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 0/4] clk: mvebu: fix clk init order

Hi,

El 17/02/14 12:04, Gregory CLEMENT escribió:
(snip)
> 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@...20 {
> 				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 can see your point now, but as this is completely fictional, I'd say 
it's irrelevant. You can just add the names if Marvell ever makes a chip 
that sources the gates from the second coreclk. As far as I can see on 
the device trees in Linux, all mvebu hardware always sources them from 
tclk. Don't try to over-engineer your driver for something that is 
unlikely to happen in reality.

If you in the future need to support another legacy platform with a 
half-cooked DT not listing the names, you can always list the right 
parent on the divisor table (see link for example) and override the default.

http://lxr.free-electrons.com/source/drivers/clk/mvebu/kirkwood.c?v=3.13#L222

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

It's not a regression if things don't break :-)

Cheers,

Emilio
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/