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

2012-05-24 Thread Paul Walmsley
Hi

On Wed, 23 May 2012, Hiremath, Vaibhav wrote:

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

No, on OMAP3, most modules need both the interface clock enabled, and at 
least one of their functional clocks.  For modules with multiple 
functional clocks like the OMAP3 USBHOST, we had to use trial 
and error to determine which functional clock was the main clock, since it 
wasn't documented.  If we got it wrong, then register accesses to the 
module would result in an abort.

The AM33xx/OMAP4 behavior should be a little easier in this regard, 
though, since the MODULEMODE bits should just control everything for a 
given module, and that should be handled by the hwmod code.  Nevertheless 
it is still good to specify a main_clk so we know how fast the module's 
registers are clocked and to locate the module in the power management 
hierarchy.

 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.

Could you please provide an example?

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

Well it definitely should be present as the ocp_if.clk.  I personally 
think it would be good to list the same clock as the hwmod's main_clk too, 
but it's currently not strictly necessary.  There are some examples in the 
omap_hwmod_44xx_data.c file, like omap44xx_mailbox_hwmod.


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-05-24 Thread Hiremath, Vaibhav
On Thu, May 24, 2012 at 13:01:11, Paul Walmsley wrote:
 Hi
 
 On Wed, 23 May 2012, Hiremath, Vaibhav wrote:
 
  I believe register read/write to IP block is depends on only interface 
  Clocks? Atleast in case of OMAP3, it was like that, right??
 
 No, on OMAP3, most modules need both the interface clock enabled, and at 
 least one of their functional clocks.  For modules with multiple 
 functional clocks like the OMAP3 USBHOST, we had to use trial 
 and error to determine which functional clock was the main clock, since it 
 wasn't documented.  If we got it wrong, then register accesses to the 
 module would result in an abort.
 

Ok, thanks for the clarification.

 The AM33xx/OMAP4 behavior should be a little easier in this regard, 
 though, since the MODULEMODE bits should just control everything for a 
 given module, and that should be handled by the hwmod code.  Nevertheless 
 it is still good to specify a main_clk so we know how fast the module's 
 registers are clocked and to locate the module in the power management 
 hierarchy.
 
  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.
 
 Could you please provide an example?
 

In case of AM33xx, clock architecture is,

sysclk1 - L4_wakeup - wakeup domain modules

sysclk1 - L4 HS - L4 HS domain modules

sysclk1 - L4 LS - L4 LS domain modules

and so on...


Now with leaf node cleanup, we are moving upward in the clocktree, more 
close to dpll output, and is the issue related to clockdomain.


   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...
 
 Well it definitely should be present as the ocp_if.clk.  I personally 
 think it would be good to list the same clock as the hwmod's main_clk too, 
 but it's currently not strictly necessary.  There are some examples in the 
 omap_hwmod_44xx_data.c file, like omap44xx_mailbox_hwmod.
 
 

omap44xx_mailbox_hwmod doesn't have main_clk exported in the hwmod_data,
so I think I should also follow same thing, right?


The cleaned clocktree (without common-clock fw) and hwmod code is ready, I 
just need to rebase against Kevin's hwmod cpu_is_xxx cleanup series (was 
pending in last version). This shouldn't impact anything to above clocktree 
and hwmod patch.

You can access the code at - 
https://github.com/hvaibhav/am335x-linux   am335x-upstream-staging


Thanks,
Vaibhav

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-05-24 Thread Paul Walmsley
On Thu, 24 May 2012, Hiremath, Vaibhav wrote:

 On Thu, May 24, 2012 at 13:01:11, Paul Walmsley wrote:
  On Wed, 23 May 2012, Hiremath, Vaibhav wrote:
  
   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.
  
  Could you please provide an example?
 
 In case of AM33xx, clock architecture is,
 
 sysclk1 - L4_wakeup - wakeup domain modules
 
 sysclk1 - L4 HS - L4 HS domain modules
 
 sysclk1 - L4 LS - L4 LS domain modules
 
 and so on...
 
 
 Now with leaf node cleanup, we are moving upward in the clocktree, more 
 close to dpll output, and is the issue related to clockdomain.

I don't really understand.  Perhaps you could provide an example from one 
of the modules?

Are you saying that you have a module that should be in a different 
clockdomain than the clockdomain of the module's main functional clock?

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...
  
  Well it definitely should be present as the ocp_if.clk.  I personally 
  think it would be good to list the same clock as the hwmod's main_clk too, 
  but it's currently not strictly necessary.  There are some examples in the 
  omap_hwmod_44xx_data.c file, like omap44xx_mailbox_hwmod.
 
 omap44xx_mailbox_hwmod doesn't have main_clk exported in the hwmod_data,
 so I think I should also follow same thing, right?

Well please start with specifying the main_clk for all IP blocks.  It is 
potentially ambiguous not to specify it, so unless there is some problem 
with specifying it, I'd prefer to have them.

 You can access the code at - 
 https://github.com/hvaibhav/am335x-linux   am335x-upstream-staging

I'll wait until it's posted to the lists.


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-05-23 Thread Hiremath, Vaibhav
On Wed, Apr 25, 2012 at 21:06:43, Paul Walmsley wrote:
 On Wed, 25 Apr 2012, Cousson, Benoit wrote:
 
  Please take care of changing the hwmod main_clk as well. But maybe 
  that's not part of that series.
 
 It's not part of the series yet.
 
 Vaibhav, could you take care of changing the main_clk in your hwmod data 
 patches, and send those to the list?
 
 

Paul,

Just an update on the status,

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.

I will start working on migrating to common clock framework now, and hoping 
to finish it soon.


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


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.

3. If I understand correctly, hwmod_data.main_clk is functional clock and
   hwmod_ocp_if.clk is interface clock, right?

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

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-05-23 Thread Paul Walmsley
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 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.

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?

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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-05-23 Thread 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
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-30 Thread Hiremath, Vaibhav
On Sat, Apr 28, 2012 at 05:34:53, Paul Walmsley wrote:
 On Fri, 27 Apr 2012, Hiremath, Vaibhav wrote:
 
  On Thu, Apr 26, 2012 at 14:19:28, Paul Walmsley wrote:
   looking at Table 1-1 Device Features of SPRUH73D, it seems that some 
   AM33xx family chips come with some features disabled, such as the 
   PRU-ICSS, the SGX, Ethernet, or USB.  How will this affect the clock 
   tree?  
   For example, is it correct for us to include the PRU-ICSS clock control 
   on 
   a clock tree that is booted on an AM3352 or an AM3354?
  
  I thought of this, but if you look at the existing way of handling such 
  Si version based clock registration is supported by cpu_clkflg, like 
  CK_3XXX, CK_AM35XX, etc...
  
  During my baseport patch submission, Tony had removed CK_AM33XX flag while 
  merging them. I believe, we are trying to get rid of these flags, and 
  that's 
  where Tony had removed them form my baseport patch.
  I was not sure on how to handle this, so I decided to continue with full 
  chip support (AM3358 and AM3359).
 
 Okay, no problem.  Sounds like we should start with full support and then 
 remove clocks later if they cause problems on the lower-end devices.
 
 

That's my plan too.

Thanks,
Vaibhav

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-27 Thread Hiremath, Vaibhav
On Thu, Apr 26, 2012 at 14:19:28, Paul Walmsley wrote:
 Hello Vaibhav,
 
 looking at Table 1-1 Device Features of SPRUH73D, it seems that some 
 AM33xx family chips come with some features disabled, such as the 
 PRU-ICSS, the SGX, Ethernet, or USB.  How will this affect the clock tree?  
 For example, is it correct for us to include the PRU-ICSS clock control on 
 a clock tree that is booted on an AM3352 or an AM3354?
 
 

I thought of this, but if you look at the existing way of handling such Si 
version based clock registration is supported by cpu_clkflg, like CK_3XXX, 
CK_AM35XX, etc...

During my baseport patch submission, Tony had removed CK_AM33XX flag while 
merging them. I believe, we are trying to get rid of these flags, and that's 
where Tony had removed them form my baseport patch.
I was not sure on how to handle this, so I decided to continue with full 
chip support (AM3358 and AM3359).


Thanks,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-27 Thread Paul Walmsley
On Fri, 27 Apr 2012, Hiremath, Vaibhav wrote:

 On Thu, Apr 26, 2012 at 14:19:28, Paul Walmsley wrote:
  looking at Table 1-1 Device Features of SPRUH73D, it seems that some 
  AM33xx family chips come with some features disabled, such as the 
  PRU-ICSS, the SGX, Ethernet, or USB.  How will this affect the clock tree?  
  For example, is it correct for us to include the PRU-ICSS clock control on 
  a clock tree that is booted on an AM3352 or an AM3354?
 
 I thought of this, but if you look at the existing way of handling such 
 Si version based clock registration is supported by cpu_clkflg, like 
 CK_3XXX, CK_AM35XX, etc...
 
 During my baseport patch submission, Tony had removed CK_AM33XX flag while 
 merging them. I believe, we are trying to get rid of these flags, and that's 
 where Tony had removed them form my baseport patch.
 I was not sure on how to handle this, so I decided to continue with full 
 chip support (AM3358 and AM3359).

Okay, no problem.  Sounds like we should start with full support and then 
remove clocks later if they cause problems on the lower-end devices.


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-26 Thread Hiremath, Vaibhav
On Thu, Apr 26, 2012 at 11:15:21, Paul Walmsley wrote:
 Hi,
 
  +/*
  + * clkdiv32 is generated from fixed division of 732.4219
  + */
  +static struct clk clkdiv32k_ick = {
  +   .name   = clkdiv32k_ick,
  +   .clkdm_name = clk_24mhz_clkdm,
  +   .rate   = 32768,
  +   .parent = clk_24mhz,
  +   .enable_reg = AM33XX_CM_PER_CLKDIV32K_CLKCTRL,
  +   .enable_bit = AM33XX_MODULEMODE_SWCTRL,
  +   .ops= clkops_omap2_dflt,
  +};
 
 While working on this file, this clock seemed quite perplexing.  Perhaps 
 you might be able to answer some questions about it.
 
 - The fractional fixed division seems a little bogus.  Is this actually an 
 M,N counter?  A few moments with PARI revealed a likely M,N of 64,46875.  
 Could you please confirm that this is the case for this clock?
 
 - This clock structure makes this clock appear to be a fixed-frequency 
 clock.  But according to SPRUH73D Figure 8-10 Peripheral PLL Structure, 
 the divider feeding this clock can be switched between an M,N of 64,46875 
 and 32,46875 depending on the value of CONTROL_CLK32K_DIVRATIO_CTRL.  So 
 shouldn't we implement that?
 

Unfortunately, this is fractional divider, assuming input clock of 24MHz.

24MHz / 732.4219 = 32KHz (Normal OPP100)
24MHz / 366.2109 = 32KHz (OPP50)

The register control CONTROL_CLK32K_DIVRATIO_CTRL is provided to choose 
between these two dividers.

As per the Spec, we have some known HW/Si issues to run whole system at 
OPP50, that's where if you refer to the TRM Table 8-22. Per PLL Typical 
Frequencies (MHz), it only talks about OPP100, which is 732.4219 divider.

So, technically, if both OPP's (OPP100 and OPP50) works fine, then yes, we 
should handle it. But given the fact that, OPP50 is not fully functional due 
to HW/Si issues, from CORE and PER perspective we will always stay at OPP100.

To summarize, I had to make it fixed-frequency due to,

 - Fractional divisor value.
 - We will always stay at OPP100 in any case, making this divisor constant 
   always.

 - This clock is feeding downstream clocks, but it's using the MODULEMODE 
 control interface, as if it were a standalone IP block.  Do you know why 
 it's using the MODULEMODE control method rather than some optional clock 
 control bits, like OMAP4 does?
 

Yes, this clock is feeding to timers, gpio debounce clocks, etc...
Honestly, I do not know the answer to why part of it.
(May be another integration/design issue)

Thanks,
Vaibhav

 
 - Paul
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-26 Thread Paul Walmsley

Hi Vaibhav, Afzal, Vaibhav,

while working on clock33xx_data.c, it became clear that we would not be 
able to remove the MODULEMODE leaf clocks for several IP blocks that share 
driver code with other DaVinci chips.  This is because:

1. the drivers for these IP blocks only use the clock framework, rather 
than runtime PM;

and 

2. the clk_get() calls in those drivers do not specify a con_id 
parameter, which could cause problems with the clkdev clk_find()'s fuzzy 
matching algorithm.

So perhaps your group can work on converting the drivers to use runtime 
PM?  Discussing this with Kevin, it sounds like DaVinci doesn't have 
runtime PM hooks implemented, but you can probably use OMAP1 as an example 
of how to do it.

And then if clk_get() is still needed in the drivers, a specific con_id 
should be supplied, such as fck, which can also be supplied by the OMAP 
codebase.


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-26 Thread Paul Walmsley
On Thu, 26 Apr 2012, Hiremath, Vaibhav wrote:

 Unfortunately, this is fractional divider, assuming input clock of 24MHz.
 
 24MHz / 732.4219 = 32KHz (Normal OPP100)
 24MHz / 366.2109 = 32KHz (OPP50)

Yes, I already saw that in the TRM.  The question is, how is the 
fractional divider implemented in the hardware?  Is it implemented as an 
M,N counter?  It's rather unlikely to be dividing by 732.4219 directly -- 
in fact that value isn't even accurate.

 As per the Spec, we have some known HW/Si issues to run whole system at 
 OPP50, that's where if you refer to the TRM Table 8-22. Per PLL Typical 
 Frequencies (MHz), it only talks about OPP100, which is 732.4219 divider.
 
 So, technically, if both OPP's (OPP100 and OPP50) works fine, then yes, we 
 should handle it. But given the fact that, OPP50 is not fully functional due 
 to HW/Si issues, from CORE and PER perspective we will always stay at OPP100.

I see.

 To summarize, I had to make it fixed-frequency due to,
 
  - Fractional divisor value.

This could be implemented as an M,N counter, unless the hardware is doing 
something more unusual.

  - This clock is feeding downstream clocks, but it's using the MODULEMODE 
  control interface, as if it were a standalone IP block.  Do you know why 
  it's using the MODULEMODE control method rather than some optional clock 
  control bits, like OMAP4 does?
 
 Yes, this clock is feeding to timers, gpio debounce clocks, etc...
 Honestly, I do not know the answer to why part of it.
 (May be another integration/design issue)

Could you please check with the AM335x PRCM designers about this to find 
out what that MODULEMODE bit is actually doing (i.e., is it actually 
enabling the clock, and if so, why did they not use the OPTFCLKEN bits) ?  
The AM335x TRM has almost no documentation on what those MODULEMODE bits 
are doing in this case.

We're trying to remove all of these MODULEMODE clocks from the clock 
framework, and also remove the accompanying clockdomain control sequence 
from the clock code.  But now it's looking like it may be impossible due 
to this clock, which means we'll need to keep that code around, which is 
unfortunate.


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-26 Thread Hiremath, Vaibhav
On Thu, Apr 26, 2012 at 12:06:00, Paul Walmsley wrote:
 On Thu, 26 Apr 2012, Hiremath, Vaibhav wrote:
 
  Unfortunately, this is fractional divider, assuming input clock of 24MHz.
  
  24MHz / 732.4219 = 32KHz (Normal OPP100)
  24MHz / 366.2109 = 32KHz (OPP50)
 
 Yes, I already saw that in the TRM.  The question is, how is the 
 fractional divider implemented in the hardware?  Is it implemented as an 
 M,N counter?  It's rather unlikely to be dividing by 732.4219 directly -- 
 in fact that value isn't even accurate.
 

Not sure how it is implemented in HW, in any case we do not have control 
over it. The control is only provided using,
- MODULEMODE (module enable/disable)
- CONTROL_CLK32K_DIVRATIO_CTRL (divisor selection)


  As per the Spec, we have some known HW/Si issues to run whole system at 
  OPP50, that's where if you refer to the TRM Table 8-22. Per PLL Typical 
  Frequencies (MHz), it only talks about OPP100, which is 732.4219 divider.
  
  So, technically, if both OPP's (OPP100 and OPP50) works fine, then yes, we 
  should handle it. But given the fact that, OPP50 is not fully functional 
  due 
  to HW/Si issues, from CORE and PER perspective we will always stay at 
  OPP100.
 
 I see.
 
  To summarize, I had to make it fixed-frequency due to,
  
   - Fractional divisor value.
 
 This could be implemented as an M,N counter, unless the hardware is doing 
 something more unusual.
 
   - This clock is feeding downstream clocks, but it's using the MODULEMODE 
   control interface, as if it were a standalone IP block.  Do you know why 
   it's using the MODULEMODE control method rather than some optional clock 
   control bits, like OMAP4 does?
  
  Yes, this clock is feeding to timers, gpio debounce clocks, etc...
  Honestly, I do not know the answer to why part of it.
  (May be another integration/design issue)
 
 Could you please check with the AM335x PRCM designers about this to find 
 out what that MODULEMODE bit is actually doing (i.e., is it actually 
 enabling the clock, and if so, why did they not use the OPTFCLKEN bits) ?  
 The AM335x TRM has almost no documentation on what those MODULEMODE bits 
 are doing in this case.
 

I can check, I am certain that, the answer would be to consider this as a 
separate independent module. And same as other modules, control is provided 
to the SW.

 We're trying to remove all of these MODULEMODE clocks from the clock 
 framework, and also remove the accompanying clockdomain control sequence 
 from the clock code.  But now it's looking like it may be impossible due 
 to this clock, which means we'll need to keep that code around, which is 
 unfortunate.
 

I think we have to live with it.

Thanks,
Vaibhav
 
 - Paul
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-26 Thread Hiremath, Vaibhav
On Thu, Apr 26, 2012 at 11:54:51, Paul Walmsley wrote:
 
 Hi Vaibhav, Afzal, Vaibhav,
 
 while working on clock33xx_data.c, it became clear that we would not be 
 able to remove the MODULEMODE leaf clocks for several IP blocks that share 
 driver code with other DaVinci chips.  This is because:
 
 1. the drivers for these IP blocks only use the clock framework, rather 
 than runtime PM;
 
 and 
 
 2. the clk_get() calls in those drivers do not specify a con_id 
 parameter, which could cause problems with the clkdev clk_find()'s fuzzy 
 matching algorithm.
 
 So perhaps your group can work on converting the drivers to use runtime 
 PM?  Discussing this with Kevin, it sounds like DaVinci doesn't have 
 runtime PM hooks implemented, but you can probably use OMAP1 as an example 
 of how to do it.
 
 And then if clk_get() is still needed in the drivers, a specific con_id 
 should be supplied, such as fck, which can also be supplied by the OMAP 
 codebase.
 
 

Honestly, I don't want to block AM33xx stuff getting into mainline due to 
missing feature in Davinci.

Also, since AM33xx is not fully supported in the mainline (from peripheral 
perspective), it really should not break anything as of now.

I am expecting, AM33xx core patches will get in by v3.5 and then while 
adding peripheral support we will take Davinci migration activity, and can
make sure that nothing breaks (for all Davinci, OMAP and AM33xx).

Thanks,
Vaibhav

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-26 Thread Paul Walmsley
On Thu, 26 Apr 2012, Hiremath, Vaibhav wrote:

 On Thu, Apr 26, 2012 at 12:06:00, Paul Walmsley wrote:
  On Thu, 26 Apr 2012, Hiremath, Vaibhav wrote:
  
   Unfortunately, this is fractional divider, assuming input clock of 24MHz.
   
   24MHz / 732.4219 = 32KHz (Normal OPP100)
   24MHz / 366.2109 = 32KHz (OPP50)
  
  Yes, I already saw that in the TRM.  The question is, how is the 
  fractional divider implemented in the hardware?  Is it implemented as an 
  M,N counter?  It's rather unlikely to be dividing by 732.4219 directly -- 
  in fact that value isn't even accurate.
 
 Not sure how it is implemented in HW, in any case we do not have control 
 over it. The control is only provided using,
   - MODULEMODE (module enable/disable)
   - CONTROL_CLK32K_DIVRATIO_CTRL (divisor selection)

Right, but the goal is to make the clock tree rate computation independent 
of the assumptions of the OPP settings.

So placing a fixed clock rate -- one that supposedly does not depend on a 
parent rate -- in the middle of a clock tree is something we wish to 
avoid, if possible.  It is a hack.  Especially considering that the 
clock's input rate does appear to depend on its parent's rate.

So, could you please double-check to find out if the appropriate 
abstraction is as an M,N counter?  If the public forum is an issue, then 
if you prefer, you can E-mail me privately with the results.

- This clock is feeding downstream clocks, but it's using the 
MODULEMODE 
control interface, as if it were a standalone IP block.  Do you know 
why 
it's using the MODULEMODE control method rather than some optional 
clock 
control bits, like OMAP4 does?
   
   Yes, this clock is feeding to timers, gpio debounce clocks, etc...
   Honestly, I do not know the answer to why part of it.
   (May be another integration/design issue)
  
  Could you please check with the AM335x PRCM designers about this to find 
  out what that MODULEMODE bit is actually doing (i.e., is it actually 
  enabling the clock, and if so, why did they not use the OPTFCLKEN bits) ?  
  The AM335x TRM has almost no documentation on what those MODULEMODE bits 
  are doing in this case.
 
 I can check, I am certain that, the answer would be to consider this as a 
 separate independent module.

Yes, please check with the hardware team.  The questions would be:

- What does changing the CLKDIV32K_CLKCTRL.MODULEMODE bits do?

- Is it necessary for the MODULEMODE bits to be set to 2 (ENABLE) to 
generate a clock to the clock nodes downstream of CLKDIV32K, or does the 
MODULEMODE just affect some other register access by the MPU?

- Will setting the CLKDIV32K_CLKCTRL.MODULEMODE bits to 0 disable the 
clock from being provided to downstream nodes of the CLKDIV32K, or does it 
simply affect register access?

- Is it necessary to follow the clockdomain enable sequence before 
enabling the CLKDIV32K MODULEMODE?

- What clockdomain does the CLKDIV32K module belong to?

 And same as other modules, control is provided to the SW.

The problem is that CLKDIV32K does not appear to be a distinct, leaf IP 
block from a clocking perspective, unlike the other uses of the 
*_CLKCTRL.MODULEMODE bits in the AM335x and OMAP4 clock trees.  So 
something is very unusual here.

If one of our core framework assumptions for OMAP4+ devices is incorrect, 
it would be useful for us to know as soon as possible, to avoid churn and 
broken code.


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-26 Thread Paul Walmsley
On Wed, 25 Apr 2012, Hiremath, Vaibhav wrote:

 On Wed, Apr 25, 2012 at 19:25:29, Paul Walmsley wrote:
  On Wed, 25 Apr 2012, Hiremath, Vaibhav wrote:
  
   Thanks for describing it for me. I will change AM33XX clock tree for this
   And submit the next version soon.
  
  Well I can just remove those leaf clock entries from my copy here, if 
  you're okay with that ?
  
 
 Great...
 
 More that OK :)

I have taken a first pass at this.  The updated patch is below.  It has 
been compile-tested only.  Could you please review this and try testing 
it?  It is also in the branch 'am33xx_support_3.5' of 
git://git.pwsan.com/linux-2.6.

Here are a few notes:

- Leaf nodes with MODULEMODE clock enable bits were removed, where there 
were no obvious driver dependencies.  Removing these may not be the 
correct thing to do, considering strange clocks like the CLKDIV32K.  It 
may be that the underlying clockdomain control for this device is very 
different than the OMAP4, and if so, we need to find this out sooner 
rather than later.

- Clock nodes with no direct hardware control have been dropped.  This has 
the unfortunate consequence of making the clock tree slightly harder to 
follow, but, in combination with the MODULEMODE clock removals, reduces 
the diffstat burden by about 1000 lines.

- Many clksel clocks were missing .init, .set_rate, and .round_rate 
function pointers; these have been added where it appeared to be 
appropriate.  Also, many clksel clocks had incorrect .recalc function 
pointers that did not take the clksel fields into consideration; these 
have been fixed.

- Several common struct clksel_rate blocks were shared with the OMAP4 
clock tree.

- The non-inlineable function am33xx_init_timer_parent() was moved from
clock33xx.h to clock33xx.c.

- Some multi-line comment formatting and mach-omap2/control.h formatting 
was cleaned up.

- AM33XX_CONTROL_STATUS_SYSBOOT1_MASK/SHIFT macros were added to remove 
a magic number from the clock tree.

- The clk_sel nomenclature in the original patch was changed to clksel 
to conform with the use in OMAP2/3/4 clock trees.

- The timer mux clock nodes have been split from the timer MODULEMODE 
nodes.

Still to be done:

* Some clocks seem to be missing clockdomain names.  These probably should 
be added.

* The clkdiv32k clock needs to be revised, depending on feedback from the 
hardware team.


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-26 Thread Paul Walmsley
On Thu, 26 Apr 2012, Paul Walmsley wrote:

 I have taken a first pass at this.  The updated patch is below.  It has 
 been compile-tested only.  Could you please review this and try testing 
 it?  It is also in the branch 'am33xx_support_3.5' of 
 git://git.pwsan.com/linux-2.6.

Here is the patch.


- Paul

From: Vaibhav Hiremath hvaib...@ti.com
Date: Wed, 18 Apr 2012 15:46:53 -0600
Subject: [PATCH] ARM: OMAP3+: clock33xx: Add AM33XX clock tree data

AM33XX clock implementation is different than any existing OMAP
family of devices. Although DPLL module is similar to OMAP4
device, but the usage is very much different than OMAP4.
AM33XX has different peripheral set and each module gets
integrated to the clock framework differently than OMAP
family of devices.

This patch adds full Clock tree data for AM33XX family
of devices and also integrates it into existing OMAP framework.

Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Afzal Mohammed af...@ti.com
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Cc: Kevin Hilman khil...@ti.com
Cc: Rajendra Nayak rna...@ti.com
CC: Tony Lindgren t...@atomide.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Benoit Cousson b-cous...@ti.com
[p...@pwsan.com: renamed clk_sel to clksel to conform with other
 OMAPs; dropped leaf modulemode clock nodes; dropped empty interface
 clock nodes; share common clksel_rate blocks between OMAP44xx and
 AM33xx; added AM33XX_CONTROL_STATUS_SYSBOOT1_MASK/SHIFT macros; fixed
 some control.h formatting; fixed some multiline comment formatting;
 moved the non-inlineable function am33xx_init_timer_parent() from
 clock33xx.h to clock33xx.c; added .set_rate  .round_rate function pointers
 for many clksel clocks; added .init function pointer to many clksel clocks;
 changed .recalc function pointer for many clksel clocks; timer fcks
 have been split from timer mux control clocks]
---
 arch/arm/mach-omap2/Makefile  |1 +
 arch/arm/mach-omap2/clock.h   |   14 +
 arch/arm/mach-omap2/clock33xx_data.c  | 1159 +
 arch/arm/mach-omap2/clock3xxx_data.c  |   20 +-
 arch/arm/mach-omap2/clock44xx_data.c  |   72 --
 arch/arm/mach-omap2/clock_common_data.c   |   77 ++
 arch/arm/mach-omap2/control.h |   32 +-
 arch/arm/mach-omap2/io.c  |3 +-
 arch/arm/plat-omap/include/plat/clkdev_omap.h |1 +
 9 files changed, 1274 insertions(+), 105 deletions(-)
 create mode 100644 arch/arm/mach-omap2/clock33xx_data.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 0fb1eaf..c20498f 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -156,6 +156,7 @@ obj-$(CONFIG_ARCH_OMAP3)+= $(clock-common) 
clock3xxx.o \
   clock3517.o clock36xx.o \
   dpll3xxx.o clock3xxx_data.o \
   clkt_iclk.o
+obj-$(CONFIG_SOC_OMAPAM33XX)   += clock33xx_data.o
 obj-$(CONFIG_ARCH_OMAP4)   += $(clock-common) clock44xx_data.o \
   dpll3xxx.o dpll44xx.o
 
diff --git a/arch/arm/mach-omap2/clock.h b/arch/arm/mach-omap2/clock.h
index b8c2a68..afb0eb4 100644
--- a/arch/arm/mach-omap2/clock.h
+++ b/arch/arm/mach-omap2/clock.h
@@ -163,4 +163,18 @@ extern const struct clkops clkops_omap3_noncore_dpll_ops;
 extern const struct clkops clkops_omap3_core_dpll_ops;
 extern const struct clkops clkops_omap4_dpllmx_ops;
 
+/* clksel_rate blocks shared between OMAP44xx and AM33xx */
+extern const struct clksel_rate div_1_0_rates[];
+extern const struct clksel_rate div_1_1_rates[];
+extern const struct clksel_rate div_1_2_rates[];
+extern const struct clksel_rate div_1_3_rates[];
+extern const struct clksel_rate div_1_4_rates[];
+extern const struct clksel_rate div31_1to31_rates[];
+
+/* clocks shared between various OMAP SoCs */
+extern struct clk virt_1920_ck;
+extern struct clk virt_2600_ck;
+
+extern int am33xx_clk_init(void);
+
 #endif
diff --git a/arch/arm/mach-omap2/clock33xx_data.c 
b/arch/arm/mach-omap2/clock33xx_data.c
new file mode 100644
index 000..02854e2
--- /dev/null
+++ b/arch/arm/mach-omap2/clock33xx_data.c
@@ -0,0 +1,1159 @@
+/*
+ * AM33XX Clock data
+ *
+ * Copyright (C) 2011-2012 Texas Instruments Incorporated - http://www.ti.com/
+ * Vaibhav Hiremath hvaib...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed as is WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/list.h
+#include linux/clk.h
+#include 

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

2012-04-26 Thread Paul Walmsley
Hello Vaibhav,

looking at Table 1-1 Device Features of SPRUH73D, it seems that some 
AM33xx family chips come with some features disabled, such as the 
PRU-ICSS, the SGX, Ethernet, or USB.  How will this affect the clock tree?  
For example, is it correct for us to include the PRU-ICSS clock control on 
a clock tree that is booted on an AM3352 or an AM3354?


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Cousson, Benoit

Hi Vaibhav,

On 4/25/2012 7:48 AM, Hiremath, Vaibhav wrote:

On Wed, Apr 25, 2012 at 06:33:26, Paul Walmsley wrote:

Hello Vaibhav, Afzal, Vaibhav,

On Tue, 3 Apr 2012, Vaibhav Hiremath wrote:


AM33XX clock implementation is different than any existing OMAP
family of devices. Although DPLL module is similar to OMAP4
device, but the usage is very much different than OMAP4.
AM33XX has different peripheral set and each module gets
integrated to the clock framework differently than OMAP
family of devices.

This patch adds full Clock tree data for AM33XX family
of devices and also integrates it into existing OMAP framework.


What do you think about the possibility of removing all of the leaf clocks
that have AM33XX_MODULEMODE_SWCTRL as their .enable_bit, assuming there
are no .fixed_div or .clksel* fields associated with the clocks?

In theory, I don't think they are needed.  The drivers should be using
runtime PM, and that should enable and disable the module via the hwmod
code, rather than the clock code.

Of course some clocks would still be needed for the main_clk fields for
the hwmods, but the hwmods should be able to use the leaf clock's parent
clocks for that.

That would save quite a few lines of data.  And I think Benoît is planning
to do that for OMAP4+.

What do you think?



Paul,

Yes, theoretically it is possible to do it. But it will also break some of
the existing things, like,

1. DebugFS clock interface

I believe, with this change, you will not have all the leaf nodes as part of 
clock tree, so they will not be populated in /debug/clock/

2. Enable and disable of the module is one part, but what about, changing
the rate of the clock (followparent_recalc)?
With the proper and complete clock tree, you can traverse the clock and
driver code doesn't need to know about parent.
Driver can simply call clk_set_rate() on leaf clock, and clock tree will
handle it.

If at all we take this path, we have to build the clk node runtime
(on-the-fly), AND/OR add new pm_runtime_set_rate() api.


Not at all. You just have to get the fck of that hwmod and use the clock 
API.



Are you available on IRC chat anytime? We can discuss on this and align
quickly.
I am available on linux-omap irc channel (with the name = hvaibhav)


That will not change anything, the point is that MODULEMODE_SWCTRL is 
uses for module control, not for clock directly, and that's why it is 
handled by the hwmod.


That will just replace the main_clk from the hwmod with the parent of 
the current modulemode nodes. Only the enable/disable part will be 
handled, if that node used to have a div, then the parent will handle that.


Today this module mode clock node is just a duplication of the hwmod 
node. By removing it, you will reduce the size of the data and have a 
much mode accurate representation of the reality.


Using the clock tree to handle these nodes was a hack we had to do 
because the hwmod fmwk was not ready when OMAP4 was introduced and 
because most drivers were not using pm_runtime.


Regards,
Benoit
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Hiremath, Vaibhav
On Wed, Apr 25, 2012 at 14:10:49, Cousson, Benoit wrote:
 Hi Vaibhav,
 
 On 4/25/2012 7:48 AM, Hiremath, Vaibhav wrote:
  On Wed, Apr 25, 2012 at 06:33:26, Paul Walmsley wrote:
  Hello Vaibhav, Afzal, Vaibhav,
 
  On Tue, 3 Apr 2012, Vaibhav Hiremath wrote:
 
  AM33XX clock implementation is different than any existing OMAP
  family of devices. Although DPLL module is similar to OMAP4
  device, but the usage is very much different than OMAP4.
  AM33XX has different peripheral set and each module gets
  integrated to the clock framework differently than OMAP
  family of devices.
 
  This patch adds full Clock tree data for AM33XX family
  of devices and also integrates it into existing OMAP framework.
 
  What do you think about the possibility of removing all of the leaf clocks
  that have AM33XX_MODULEMODE_SWCTRL as their .enable_bit, assuming there
  are no .fixed_div or .clksel* fields associated with the clocks?
 
  In theory, I don't think they are needed.  The drivers should be using
  runtime PM, and that should enable and disable the module via the hwmod
  code, rather than the clock code.
 
  Of course some clocks would still be needed for the main_clk fields for
  the hwmods, but the hwmods should be able to use the leaf clock's parent
  clocks for that.
 
  That would save quite a few lines of data.  And I think Benoît is planning
  to do that for OMAP4+.
 
  What do you think?
 
 
  Paul,
 
  Yes, theoretically it is possible to do it. But it will also break some of
  the existing things, like,
 
  1. DebugFS clock interface
 
  I believe, with this change, you will not have all the leaf nodes as part 
  of clock tree, so they will not be populated in /debug/clock/
 
  2. Enable and disable of the module is one part, but what about, changing
  the rate of the clock (followparent_recalc)?
  With the proper and complete clock tree, you can traverse the clock and
  driver code doesn't need to know about parent.
  Driver can simply call clk_set_rate() on leaf clock, and clock tree will
  handle it.
 
  If at all we take this path, we have to build the clk node runtime
  (on-the-fly), AND/OR add new pm_runtime_set_rate() api.
 
 Not at all. You just have to get the fck of that hwmod and use the clock 
 API.
 
  Are you available on IRC chat anytime? We can discuss on this and align
  quickly.
  I am available on linux-omap irc channel (with the name = hvaibhav)
 
 That will not change anything, the point is that MODULEMODE_SWCTRL is 
 uses for module control, not for clock directly, and that's why it is 
 handled by the hwmod.
 
 That will just replace the main_clk from the hwmod with the parent of 
 the current modulemode nodes. Only the enable/disable part will be 
 handled, if that node used to have a div, then the parent will handle that.
 
 Today this module mode clock node is just a duplication of the hwmod 
 node. By removing it, you will reduce the size of the data and have a 
 much mode accurate representation of the reality.
 
 Using the clock tree to handle these nodes was a hack we had to do 
 because the hwmod fmwk was not ready when OMAP4 was introduced and 
 because most drivers were not using pm_runtime.
 

Benoit,

I completely understand what are trying to explain here, and I completely agree 
with you on the fact that, MODULEMODE_SWCTRL is for module control and is clock 
node is just a duplication. 

Module enable and disable is one part, and I am not at all referring to this.
My point is, more from other piece of code which are dependent on clocktree.
In order to have proper and complete debugfs representation of all the 
clocks, somebody has to maintain the information of leaf clocks to parent 
relation, Isn't it?

For example, take OMAP44xx dss_dss_clk clock, OR any clock with 
followparent_recalc field, the question I am raising here is,

How would I know the rate of this clock in driver? Say for example, I want 
to configure my internal divider based on input rate? 
OR 
I want to change the rate of dss_clk itself?
OR
Looking at debugfs, would I get the rate of dss_clk? 
OR
Will I have dss_clk present in /debug/clock/?
OR 
Are we expecting user to know parent of such leaf clocks?
OR
Are we planning to export hwmod-main_clk (which is parent clk here) to
User in debugfs or something?

Thanks,
Vaibhav
 Regards,
 Benoit
 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Cousson, Benoit

On 4/25/2012 12:20 PM, Hiremath, Vaibhav wrote:

On Wed, Apr 25, 2012 at 14:10:49, Cousson, Benoit wrote:


...


That will not change anything, the point is that MODULEMODE_SWCTRL is
uses for module control, not for clock directly, and that's why it is
handled by the hwmod.

That will just replace the main_clk from the hwmod with the parent of
the current modulemode nodes. Only the enable/disable part will be
handled, if that node used to have a div, then the parent will handle that.

Today this module mode clock node is just a duplication of the hwmod
node. By removing it, you will reduce the size of the data and have a
much mode accurate representation of the reality.

Using the clock tree to handle these nodes was a hack we had to do
because the hwmod fmwk was not ready when OMAP4 was introduced and
because most drivers were not using pm_runtime.



Benoit,

I completely understand what are trying to explain here, and I completely agree 
with you on the fact that, MODULEMODE_SWCTRL is for module control and is clock 
node is just a duplication.

Module enable and disable is one part, and I am not at all referring to this.
My point is, more from other piece of code which are dependent on clocktree.
In order to have proper and complete debugfs representation of all the
clocks, somebody has to maintain the information of leaf clocks to parent
relation, Isn't it?


Nope, not at all. All the clocks will stay in the clock tree. The clock 
consumers a.k.a hwmods does not have to be in the clock tree.
Having some fake leaf nodes in the clock tree is just adding some fake 
intermediate clocks instead of the real consumer. These nodes do not 
exist, they were added to have a point of control from the driver before 
runtime_pm was there.



For example, take OMAP44xx dss_dss_clk clock, OR any clock with 
followparent_recalc field, the question I am raising here is,


Then it is not an issue. If this is a real clock, it will then stay in 
the clock data.



How would I know the rate of this clock in driver? Say for example, I want
to configure my internal divider based on input rate?
OR
I want to change the rate of dss_clk itself?
OR
Looking at debugfs, would I get the rate of dss_clk?


No because that clock will not exist anymore, but you will have the real 
clock rate from the dss_dss_clk.



OR
Will I have dss_clk present in /debug/clock/?
OR
Are we expecting user to know parent of such leaf clocks?


Yes, because it is not leaf nodes anymore, but modules.


OR
Are we planning to export hwmod-main_clk (which is parent clk here) to
User in debugfs or something?


Well, I used to have a patch do expose hwmod to debugfs, but this is not 
that useful.


I think this is a non-issue, we are just removing clock nodes that are 
not real nodes, so it will not change anything practically speaking.


Regards,
Benoit

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Hiremath, Vaibhav
On Wed, Apr 25, 2012 at 17:08:43, Cousson, Benoit wrote:
 On 4/25/2012 12:20 PM, Hiremath, Vaibhav wrote:
  On Wed, Apr 25, 2012 at 14:10:49, Cousson, Benoit wrote:
 
 ...
 
snip
 
  How would I know the rate of this clock in driver? Say for example, I want
  to configure my internal divider based on input rate?
  OR
  I want to change the rate of dss_clk itself?
  OR
  Looking at debugfs, would I get the rate of dss_clk?
 
 No because that clock will not exist anymore, but you will have the real 
 clock rate from the dss_dss_clk.
 

Thanks Benoit, I think I understand your perspective on Module Vs leaf node 
and now able to digest as well.
Probably Last Q, which I think clarify all my doubts,

Assume we have complete hwmod instance and built a device using 
omap_device_build() api, and also the driver is completely using runtime pm 
api's,
How can driver get the clk handle (required to get the rate)? 
Is there any api already available for this?

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Cousson, Benoit

On 4/25/2012 2:26 PM, Hiremath, Vaibhav wrote:

On Wed, Apr 25, 2012 at 17:08:43, Cousson, Benoit wrote:

On 4/25/2012 12:20 PM, Hiremath, Vaibhav wrote:

On Wed, Apr 25, 2012 at 14:10:49, Cousson, Benoit wrote:


...


snip



How would I know the rate of this clock in driver? Say for example, I want
to configure my internal divider based on input rate?
OR
I want to change the rate of dss_clk itself?
OR
Looking at debugfs, would I get the rate of dss_clk?


No because that clock will not exist anymore, but you will have the real
clock rate from the dss_dss_clk.



Thanks Benoit, I think I understand your perspective on Module Vs leaf node
and now able to digest as well.
Probably Last Q, which I think clarify all my doubts,

Assume we have complete hwmod instance and built a device using
omap_device_build() api, and also the driver is completely using runtime pm
api's,
How can driver get the clk handle (required to get the rate)?
Is there any api already available for this?


The omap_device fmwk will auto-magically create a fck clkdev entry for 
the main_clk.


The API is thus the standard: clk_get(dev, fck), to get the clock 
handle and then you can use the other clock API.


The idea is to enforce the usage of clock alias local to the device and 
not trying anymore to get the real clock name. It thus allows driver to 
work on various variant / revision without having to modify the driver.


The fmwk will also create clock alias for any opt_clock present in the 
hwmod.


Regards,
Benoit


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Hiremath, Vaibhav
On Wed, Apr 25, 2012 at 18:03:21, Cousson, Benoit wrote:
 On 4/25/2012 2:26 PM, Hiremath, Vaibhav wrote:
  On Wed, Apr 25, 2012 at 17:08:43, Cousson, Benoit wrote:
  On 4/25/2012 12:20 PM, Hiremath, Vaibhav wrote:
  On Wed, Apr 25, 2012 at 14:10:49, Cousson, Benoit wrote:
 
  ...
 
  snip
 
  How would I know the rate of this clock in driver? Say for example, I want
  to configure my internal divider based on input rate?
  OR
  I want to change the rate of dss_clk itself?
  OR
  Looking at debugfs, would I get the rate of dss_clk?
 
  No because that clock will not exist anymore, but you will have the real
  clock rate from the dss_dss_clk.
 
 
  Thanks Benoit, I think I understand your perspective on Module Vs leaf node
  and now able to digest as well.
  Probably Last Q, which I think clarify all my doubts,
 
  Assume we have complete hwmod instance and built a device using
  omap_device_build() api, and also the driver is completely using runtime pm
  api's,
  How can driver get the clk handle (required to get the rate)?
  Is there any api already available for this?
 
 The omap_device fmwk will auto-magically create a fck clkdev entry for 
 the main_clk.
 
 The API is thus the standard: clk_get(dev, fck), to get the clock 
 handle and then you can use the other clock API.
 
 The idea is to enforce the usage of clock alias local to the device and 
 not trying anymore to get the real clock name. It thus allows driver to 
 work on various variant / revision without having to modify the driver.
 
 The fmwk will also create clock alias for any opt_clock present in the 
 hwmod.
 

Yes, Yes, I missed this. Omap_device_build() internally add fck to the 
clkdev table for both main_clk and opt_clks.

Thanks for describing it for me. I will change AM33XX clock tree for this
And submit the next version soon.

Thanks,
Vaibhav

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Paul Walmsley
Hi Vaibhav,

On Wed, 25 Apr 2012, Hiremath, Vaibhav wrote:

 Thanks for describing it for me. I will change AM33XX clock tree for this
 And submit the next version soon.

Well I can just remove those leaf clock entries from my copy here, if 
you're okay with that ?


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Hiremath, Vaibhav
On Wed, Apr 25, 2012 at 19:25:29, Paul Walmsley wrote:
 Hi Vaibhav,
 
 On Wed, 25 Apr 2012, Hiremath, Vaibhav wrote:
 
  Thanks for describing it for me. I will change AM33XX clock tree for this
  And submit the next version soon.
 
 Well I can just remove those leaf clock entries from my copy here, if 
 you're okay with that ?
 

Great...

More that OK :)

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Cousson, Benoit

On 4/25/2012 3:55 PM, Paul Walmsley wrote:

Hi Vaibhav,

On Wed, 25 Apr 2012, Hiremath, Vaibhav wrote:


Thanks for describing it for me. I will change AM33XX clock tree for this
And submit the next version soon.


Well I can just remove those leaf clock entries from my copy here, if
you're okay with that ?


Please take care of changing the hwmod main_clk as well. But maybe 
that's not part of that series.


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Paul Walmsley
On Wed, 25 Apr 2012, Cousson, Benoit wrote:

 Please take care of changing the hwmod main_clk as well. But maybe 
 that's not part of that series.

It's not part of the series yet.

Vaibhav, could you take care of changing the main_clk in your hwmod data 
patches, and send those to the list?


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Hiremath, Vaibhav
On Wed, Apr 25, 2012 at 21:06:43, Paul Walmsley wrote:
 On Wed, 25 Apr 2012, Cousson, Benoit wrote:
 
  Please take care of changing the hwmod main_clk as well. But maybe 
  that's not part of that series.
 
 It's not part of the series yet.
 
 Vaibhav, could you take care of changing the main_clk in your hwmod data 
 patches, and send those to the list?
 
 
Yes, Working on the same now...

Thanks,
Vaibhav

 - Paul
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-25 Thread Paul Walmsley
Hi,

 +/*
 + * clkdiv32 is generated from fixed division of 732.4219
 + */
 +static struct clk clkdiv32k_ick = {
 + .name   = clkdiv32k_ick,
 + .clkdm_name = clk_24mhz_clkdm,
 + .rate   = 32768,
 + .parent = clk_24mhz,
 + .enable_reg = AM33XX_CM_PER_CLKDIV32K_CLKCTRL,
 + .enable_bit = AM33XX_MODULEMODE_SWCTRL,
 + .ops= clkops_omap2_dflt,
 +};

While working on this file, this clock seemed quite perplexing.  Perhaps 
you might be able to answer some questions about it.

- The fractional fixed division seems a little bogus.  Is this actually an 
M,N counter?  A few moments with PARI revealed a likely M,N of 64,46875.  
Could you please confirm that this is the case for this clock?

- This clock structure makes this clock appear to be a fixed-frequency 
clock.  But according to SPRUH73D Figure 8-10 Peripheral PLL Structure, 
the divider feeding this clock can be switched between an M,N of 64,46875 
and 32,46875 depending on the value of CONTROL_CLK32K_DIVRATIO_CTRL.  So 
shouldn't we implement that?

- This clock is feeding downstream clocks, but it's using the MODULEMODE 
control interface, as if it were a standalone IP block.  Do you know why 
it's using the MODULEMODE control method rather than some optional clock 
control bits, like OMAP4 does?


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-04-24 Thread Paul Walmsley
Hello Vaibhav, Afzal, Vaibhav,

On Tue, 3 Apr 2012, Vaibhav Hiremath wrote:

 AM33XX clock implementation is different than any existing OMAP
 family of devices. Although DPLL module is similar to OMAP4
 device, but the usage is very much different than OMAP4.
 AM33XX has different peripheral set and each module gets
 integrated to the clock framework differently than OMAP
 family of devices.
 
 This patch adds full Clock tree data for AM33XX family
 of devices and also integrates it into existing OMAP framework.

What do you think about the possibility of removing all of the leaf clocks
that have AM33XX_MODULEMODE_SWCTRL as their .enable_bit, assuming there 
are no .fixed_div or .clksel* fields associated with the clocks?

In theory, I don't think they are needed.  The drivers should be using 
runtime PM, and that should enable and disable the module via the hwmod 
code, rather than the clock code.

Of course some clocks would still be needed for the main_clk fields for 
the hwmods, but the hwmods should be able to use the leaf clock's parent 
clocks for that.

That would save quite a few lines of data.  And I think Benoît is planning 
to do that for OMAP4+.

What do you think?


- Paul

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

2012-04-24 Thread Hiremath, Vaibhav
On Wed, Apr 25, 2012 at 06:33:26, Paul Walmsley wrote:
 Hello Vaibhav, Afzal, Vaibhav,
 
 On Tue, 3 Apr 2012, Vaibhav Hiremath wrote:
 
  AM33XX clock implementation is different than any existing OMAP
  family of devices. Although DPLL module is similar to OMAP4
  device, but the usage is very much different than OMAP4.
  AM33XX has different peripheral set and each module gets
  integrated to the clock framework differently than OMAP
  family of devices.
  
  This patch adds full Clock tree data for AM33XX family
  of devices and also integrates it into existing OMAP framework.
 
 What do you think about the possibility of removing all of the leaf clocks
 that have AM33XX_MODULEMODE_SWCTRL as their .enable_bit, assuming there 
 are no .fixed_div or .clksel* fields associated with the clocks?
 
 In theory, I don't think they are needed.  The drivers should be using 
 runtime PM, and that should enable and disable the module via the hwmod 
 code, rather than the clock code.
 
 Of course some clocks would still be needed for the main_clk fields for 
 the hwmods, but the hwmods should be able to use the leaf clock's parent 
 clocks for that.
 
 That would save quite a few lines of data.  And I think Benoît is planning 
 to do that for OMAP4+.
 
 What do you think?
 

Paul,

Yes, theoretically it is possible to do it. But it will also break some of 
the existing things, like,

1. DebugFS clock interface

I believe, with this change, you will not have all the leaf nodes as part of 
clock tree, so they will not be populated in /debug/clock/

2. Enable and disable of the module is one part, but what about, changing 
   the rate of the clock (followparent_recalc)? 
   With the proper and complete clock tree, you can traverse the clock and 
   driver code doesn't need to know about parent. 
   Driver can simply call clk_set_rate() on leaf clock, and clock tree will 
   handle it.

If at all we take this path, we have to build the clk node runtime 
(on-the-fly), AND/OR add new pm_runtime_set_rate() api.

Are you available on IRC chat anytime? We can discuss on this and align 
quickly.
I am available on linux-omap irc channel (with the name = hvaibhav)

Thanks,
Vaibhav

 
 - Paul
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html