Re: [RFC][PATCH 1/2] ARM: OMAP4: clock: Add device tree support for AUXCLKs

[Roger Quadros <rogerq@ti.com>]

On 04/05/2013 08:56 PM, Grygorii Strashko wrote:
> On 04/04/2013 07:41 PM, Tony Lindgren wrote:
>> * Roger Quadros <rogerq@ti.com> [130404 00:39]:
>>> On 04/04/2013 02:42 AM, Tony Lindgren wrote:
>>>>> --- a/arch/arm/mach-omap2/cclock44xx_data.c
>>>>> +++ b/arch/arm/mach-omap2/cclock44xx_data.c
>>>>> @@ -27,6 +27,7 @@
>>>>>   #include <linux/clk-private.h>
>>>>>   #include <linux/clkdev.h>
>>>>>   #include <linux/io.h>
>>>>> +#include <linux/of.h>
>>>>>     #include "soc.h"
>>>>>   #include "iomap.h"
>>>>> @@ -1663,6 +1664,40 @@ static struct omap_clk omap44xx_clks[] = {
>>>>>       CLK(NULL,    "cpufreq_ck",    &dpll_mpu_ck,    CK_443X),
>>>>>   };
>>>>>   +static struct clk *scrm_clks[] = {
>>>>> +    &auxclk0_ck,
>>>>> +    &auxclk1_ck,
>>>>> +    &auxclk2_ck,
>>>>> +    &auxclk3_ck,
>>>>> +    &auxclk4_ck,
>>>>> +    &auxclk5_ck,
>>>>> +};
>>>> Hmm I don't like the idea of specifying the auxclk both in the
>>>> cclock44xx_data.c and in DT..
>>> Right, but till we have all clocks moved to DT we only need this
>>> approach for general purpose clocks that are not mapped to devices
>>> by hwmod.
>> For v3.10, let's just make sure that USB works with DT as then
>> after v3.10 we can make omap4 DT only and get rid of estimated
>> 7K lines of code and data. I guess this is the last piece missing
>> for that, or are we also missing something else?
>>
>> Can't you set up a clock alias for your device so it can find the
>> auxclk when requesting it with the dev entry?
>>
>> For the DT clock driver if needed for v3.10, how about just do a
>> minimal drivers/clock/omap/ that uses the standard binding?
>> Then that driver can just do clk_get() from cclock44xx_data.c
>> for now? And then later on we'll just move all the clocks to a
>> combination of DT + /lib/firmware.
>>
> Hi Roger, Rajendra, All
> 
> Sorry that disturbing you.

Hi Grygorri,

Nothing to disturb at all ;).

> 
> I'm supporting Android OMAP4 kernels (K3.0/K3.4) and like to clarify few
> points regarding this approach (having into account possible future migrations
> on newer Kernels and OMAP5).
> If I understand everything right, this patch series allows to create clock binding
> in DT using following syntax: clocks = <&aux_clks 3>

Actually in v2 of the patch this would be clocks = <&clks 3>

>  - does it means that in worst case there will be ~200 clock IDs defined?

I'm afraid so yes. But when I wrote this I was only thinking of generic clocks, i.e. AUXCLKS.
So when we start talking of all clocks we might need to reconsider the approach.

>  - does it means that clock nodes binding using phandles (human-friendly notation)
> isn't going to be supported?
> for example:
>     clocks = <&sys_clkin_ck>
>     clocks = <&dss_sys_clk &dss_tv_clk &dss_dss_clk>)

This would depend if we define the clock nodes within DT or not. From what Tony
mentioned (i.e. inefficiency) it seems that the clock nodes won't be defined
in the device tree. Then we are left with using an index.
> 
> I was horrified to think about the problems of this approach support
> (in case if there would be more then ~30 IDs) - just miss with on digit
> and weeks of debugging would be guaranteed.
> 
> Please, say me that i'm wrong.

It is still not written in stone so if you have a better idea we could
go that route.

cheers,
-roger

> And why clock DT data can't be auto-generated like all other OMAP data
> to close this questions?
> 
> Thanks.
> 
>>> e.g. auxclk are required to be specified in DT nodes for USB PHY.
>>> Without this we can't get USB host working on Panda.
>> OK. So if the USB PHY has a dev entry, can't you just set up a
>> clock alias in struct omap_clk omap44xx_clks[] for it?
>>  
>>> As Rajendra points out, it seems moving entire clock data to DT is not
>>> going to happen soon. So this is the simplistic way things can work.
>> Right but seems like we can get started there without moving
>> them all at once.
>>
>> Regards,
>>
>> Tony
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>