Felipe, Thanks for your comments. > DSPPeripheralClkCtrl is already mapping some clock ids to some other > clock ids (SERVICES_ClkId) before calling the functions in clk.c. This > clk_enable/disable can be called there. - DSPPeripheralClkCtrl is used for managing the peripheral clock requests that come from DSP. You cannot use this function to enable IVA2 and mailbox clocks. Felipe, I see your point of removing the clock module, but please don't view this just as a wrapper; it provides more functionality than just being the direct wrapper. It does hide the implementation to translate the clock id to kernel clock structure, the clock init does the job of obtaining the references to the clock ids in its initialization, and more importantly isolates the tracing to one module. There are multiple ways to implement, and yes we can remove the clock module in Bridge and move the code in these functions to other places, but in the end we will have the new code that has lesser code readability and debugging ability :). > Didn't Tony just explained how to do that without a wrapper? -- I was under the impression that Tony is thinking that the DSP Bridge is not calling kernel's clk_enable function. My above explanation holds here about the wrappers. Thank you, Best regards, Hari > -----Original Message----- > From: Felipe Contreras [mailto:felipe.contreras@xxxxxxxxx] > Sent: Tuesday, August 19, 2008 2:10 PM > To: Kanigeri, Hari > Cc: Tony Lindgren; Trilok Soni; Pasam, Vijay; Hiroshi DOYU; linux- > omap@xxxxxxxxxxxxxxx; Woodruff, Richard; siarhei.siamashka@xxxxxxxxx; > Ramirez Luna, Omar; Gupta, Ramesh > Subject: Re: [RFC][DRAFT] TODO list for TI DSP BRIDGE > > On Tue, Aug 19, 2008 at 9:24 PM, Kanigeri, Hari <h-kanigeri2@xxxxxx> > wrote: > > Hi All, > > > >> > > >> > That only makes the code harder to understand. > >> > > >> > #define clk_enable(...) my_debug_function(__VA_ARGS__) > >> > > >> > Achieves the same thing. > >> > >> We must use clk_enable() and clk_disable() in the drivers. That's the > >> Linux clock interface. Anything else won't get merged upstream. > >> > >> If you need to combine multiple clocks into a single virtual dsp > >> clock, please register a new custom clock using clk_register(). > >> > >> That allows you to do debugging in the custom clock functions too > >> if necessary. > > > > > > --- The clock manager in DSP Bridge is in charge of translating the > clock requests that come from tasks that are running on DSP to the clk > structure that is understandable to the kernel's clk_enable function. > > > > The request for clocks from DSP side of the Bridge comes in the form of > the clock ids that are defined in clk.h (The Bridge clk.h file). So, for > example, if the DSP task wants GPT6 clocks to be enabled, then a request > comes to MPU Bridge with the id for that clock. The Clock module in the > MPU Bridge does an internal lookup to translate this id to the proper > clock structure and then calls the kernel's clk_enable function with this > clock structure as the argument. So, in a way we can call this clock > module as the clock manager. > > DSPPeripheralClkCtrl is already mapping some clock ids to some other > clock ids (SERVICES_ClkId) before calling the functions in clk.c. This > clk_enable/disable can be called there. > > > One another reason for centralizing the calls to kernel's clk_enable > function in one module is to debug the cases where a task running on the > DSP is expecting some clock to be enabled but it might not have been > enabled. > > > > Example: If there is an issue with the load monitoring not working as > expected on the DSP, the first thing you may want to check is if the > clocks that are used for the load monitoring is enabled or not. The > easiest way is just enabling the traces only for clock module and based on > that you can tell if the right clocks were enabled or not. This feature > is extremely useful when debugging multimedia test cases, where there are > multiple tasks running on DSP sending requests to clocks. > > Didn't Tony just explained how to do that without a wrapper? > > Best regards. > > -- > Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
