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

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:

quoted

+
+	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:

1) both dev_id + con_id defined:
    a) search for device node named dev_id
    b) check clock-names for matching con_id
    c) return matching clock

    Example in kernel:
       clocksource-nomadik-mtu.c :
           clk_get_sys("mtu0", "apb_pclk");

       ste-nomadik-snt8815.dts:
         mtu0: mtu at 101e2000 {
                 /* Nomadik system timer */
                 compatible = "st,nomadik-mtu";
                 reg = <0x101e2000 0x1000>;
                 interrupt-parent = <&vica>;
                 interrupts = <4>;
                 clocks = <&timclk>, <&pclk>;
                 clock-names = "timclk", "apb_pclk";
         };

2) dev_id = NULL, con_id defined (current patch implementation, most of 
the OMAP clocks use this approach):
    a) search for a device node named con_id
    b) return corresponding clock from the node

Alternatively I must implement a regression and break some of the OMAP 
drivers with this set, and also implement omap internal clk_get 
functionality (basically adding a similar wrapper that I have 
implemented in this patch) under mach-omap2 to get some basic OMAP infra 
to work properly (namely, hwmod.) I can probably avoid part of this by 
adding more beef to the *.dts files for the critical parts to get boot 
working at least.

-Tero