[PATCHv5 01/31] CLK: clkdev: add support for looking up clocks from DT
t-kristo at ti.com
Mon Aug 26 14:12:28 EDT 2013
On 08/26/2013 08:03 PM, Russell King - ARM Linux wrote:
> On Mon, Aug 26, 2013 at 05:36:15PM +0300, Tero Kristo wrote:
>> On 08/03/2013 09:31 PM, Russell King - ARM Linux wrote:
>>> On Fri, Aug 02, 2013 at 07:25:20PM +0300, Tero Kristo wrote:
>>>> + if (cl)
>>>> + return cl;
>>>> + /* If clock was not found, attempt to look-up from DT */
>>>> + node = of_find_node_by_name(NULL, con_id);
>>> This is utterly broken if you're looking up purely by a connection ID.
>>> Please, go back and read clk_get()'s documentation in linux/clk.h.
>>> It's spelt out very plainly there.
>> Hi Russell,
>> After looking at this a bit more, it seems difficult to get rid of the
>> clk_get -> of_clk_get conversion, however based on this comment I am
>> somewhat stuck. Do you think it would be acceptable if I change the
>> implementation of this patch to work like this:
> Firstly, for clk_get(), you don't need to do anything - clk_get() already
> deals with DT, and if your DT is correct, it should already be returning
> the appropriate clock(s).
> I guess the problem here is clk_get_sys(), because that doesn't take a
> struct device (and it doesn't take a struct device because it's used
> from code where there is no struct device to use with it.)
Yeah, most of the OMAP clocks have been traditionally referenced in this
way, they are using clk_get_sys and with dev=NULL. conn-id is mostly unique.
> If you have an OF node, what's wrong with using of_clk_get_by_name()?
> If you don't have an OF node, how do you expect to find the clock in
> the device tree, remembering that clocks are specific to devices, and
> the 2nd argument to clk_get()/clk_get_sys() is not a unique identifier.
In the legacy code, pretty much all the clocks are mapped through
clk_get_sys(NULL, conn), and large amount of the drivers don't even have
DT conversion in place yet. I am attempting to handle the cases where we
don't have OF nodes.... yet at least, without converting everything to
DT, and without causing regressions (also, I don't even pretend to be
capable of testing all the potential drivers for this.) Some parts of
the basic infra which are using clk_get_sys are pretty silly also, like
the ocp_if parts of the hwmod, I haven't figured out yet how to convert
that to DT.
Personally I don't mind causing a regressions here though, if I get
approval for it from Tony.
More information about the linux-arm-kernel