RE: [RFC][DRAFT] TODO list for TI DSP BRIDGE

Woodruff, Richard
2008-08-20T14:33:09Z

Hi Hiroshi,

> I sent some patches based on the above suggestion.
>
> http://marc.info/?l=linux-omap&m=121922597019873&w=2
> http://marc.info/?l=linux-omap&m=121922593919766&w=2
>
> I think that this virtual clock may allow any arbitrary combination of
> clocks and it may be possible to have multiple hierarchy of virtual
> clocks like:
>
> dsp_clocks
> |
> |-- peripheral_clocks
> | |
> | |-- gpt5
> | | |- gpt5_ick
> | | `- gpt5_fck
> |
> |-- ....
> ..
>
> Any comments would be appreciated.

A few quick comments:

- First off, I do like the idea of virtual clocks for grouping. I did do this for OMAP2 for completely internal and dependent root clocks. I also thought it might be nice to extend clock frame work to allow external clock registration (say for a codec chip). In our local tree I did even add it. One main issue was the internal clock structure is supposed to be opaque and you need to set internals to add a clock node.

- I worry a bit about grouping of internal resources which may be allocated differently per implementation. Perhaps a board specific resource can handle it, but you've not included that in the patch.

- Where is the board specific side of this call? I see clock calls added to clock.c but not resources added to mach-omap2/xyz-board.c.

- You're not preserving the enable error path back to the virtual clock. A recovery from a failure would be messy.

- Will these clocks always be handled as a group? If I were to try and optimize I might shut down a gpt5 clock individually to allow a PER domain to power off. The DSP should aggressively be letting go of resource when it doesn't need them to allow power to work. 1 virtual resource with all related clocks would not be right.

If you wanted grouping their might be like DSP-CORE, DSP-Streaming-Audio, Dsp-... You will want better granularity. So the enable/disables are effective for power savings. This is especially true if your not even using say a McBSP and are doing something like JPEG decode.

Regards,
Richard W.