[PATCHv5 01/31] CLK: clkdev: add support for looking up clocks from DT

[Tero Kristo]

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.
>>>
>>> NAK.
>>>
>>
>> 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.

-Tero