[PATCH-V3 3/3] ARM: OMAP3+: clock33xx: Add AM33XX clock tree data

[Hiremath, Vaibhav]

On Wed, May 23, 2012 at 20:38:27, Paul Walmsley wrote:
> Hi Vaibhav
> 
> On Wed, 23 May 2012, Hiremath, Vaibhav wrote:
> 
> > I used your cleaned version of clocktree data, where you have removed all 
> > leaf-nodes and merged multiple clocks nodes into one; but it did not work. I 
> > attempted to review the cleanup and tried to debug, but found it bit hard to 
> > come back to working clocktree from stripped version. So moved back to my 
> > submitted clocktree and started stripping the clock leaf-nodes, same way you 
> > had done it. Now I reached to the stage where I have working clocktree 
> > without peripheral leaf-nodes and booting on BeagleBone.
> 
> Okay great, please post that to the list so we can diff it against the 
> version that I did, and also so we can queue it for merging in 3.6?
> 

Just doing final round of review and cleanup...may be by tomorrow, I should 
be able to submit it (without common-clock framework).


> > Just FYI (may need your help), I got into some issues (still open) during 
> > this cleanup -
> > 
> > 1. In cases where we have clock selection option for node-leafs, like, 
> >    Timers, I removed enable_reg entry from the node, which results into a 
> >    kernel error message from function omap2_dflt_clk_disable()
> > 
> > clock: clk_disable called on independent clock %s which has no enable_reg
> 
> Shouldn't those clocks use clkops_null then?
> 
> > 2. clkdm_name entry: 
> >    The issue is, two entries available for clockdomain associated for 
> >    module/clock 
> >         - hwmod_data.main_clk and 
> >         - clk.clkdm_name
> > 
> >    They must match, right? The parent/root clocks flows from one domain to 
> >    another domain, so I have to keep the nodes just for sake of matching 
> >    clockdomain entry present in associated main_clk data.
> > 
> >    I am still reviewing the code from Rajendra, but one of Rajendra's patch 
> >    actually will help here, made it to use hwmod_data.clkdm (if available) 
> >    always, instead of directly using clk.clkdm.
> 
> Well because of this CLKDIV32K snafu, we'll need to keep the clock 
> framework-based clockdomain control for AM33xx, at the very least for 
> the CLKDIV32K clock.
> 
> I'd suggest starting with specifying a clockdomain name for each clock.  
> If nothing else, this should help think through the power management 
> behavior for each clock.  Then those can be easily removed later with a 
> simple grep.
> 
> > 3. If I understand correctly, hwmod_data.main_clk is functional clock and    
> >    hwmod_ocp_if.clk is interface clock, right?
> 
> Basically, yes.  To be more precise, in cases where 
> modules have multiple functional clocks, hwmod_data.main_clk is the 
> functional clock that is needed in order for register reads and writes to 
> the IP block to succeed.
> 

I believe register read/write to IP block is depends on only interface 
Clocks? Atleast in case of OMAP3, it was like that, right??


Another issues is, 

I came across situation where, two modules fall into different clock domains but their functional clock is happened to be coming from same source/dpll-divider. So in order to satisfy clkdm match between them, I have
kept nodes without enable_regs.



> >    So currently, I have mentioned same entry in both the places (especially 
> >    for Peripherals/modules).
> >    I am planning to remove ocp_if/clk entry data for all modules..
> 
> Hmmm, could you explain this further?
> 

what if, module only has interface clock? Should it be present as main_clk or ocp_if.clk or both??
Example would be, TPCC, TPTC, smartreflex, etc...

Thanks,
Vaibhav

> Every ocp_if structure that you create should have an interface clock 
> specified.  And almost all of your hwmods should have a main_clk 
> specified.
> 
> 
> - Paul
>