On Mon, 2012-04-23 at 10:05 +0530, Mahapatra, Chandrabhanu wrote: > On Fri, Apr 20, 2012 at 7:59 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote: > > The patch is now otherwise fine, but I think it needs some more > > renaming. Now the code mixes fclk and core-clk names, which is rather > > confusing. I guess the calc_fclk should actually be calc_req_core_clk? > > And the fclk variable core_clk (or cclk or something)? > > > > Tomi > > > > As per TRM dispc_core_clk is also a functional clock, so I think above > code should be fine. But, to avoid confusion of names the renaming of > fclk, calc_fclk and calc_fclk_five_taps can be done. Well, that is true is a sense, if you consider a "functional clock" something that is used to drive some function of the HW. But then we could also call the pixel clock a functional clock, as it also drives some functionality. And true, TRM seems to refer to the core clock as "dispc internal functional clock" at times. But in this case, looking at the clock tree, DISPC_FCLK is the clock coming to DISPC, which is then divided by the logic clock divisor to get the logic clock (or core clock, or DISPC internal functional clock). And I think the TRM, at least the OMAP2/3 versions, mostly speak of the DISPC_FCLK as the functional clock, and that's what the driver also has meant with "functional clock". So to avoid any confusion, I'd suggest renaming them. At least I don't see any benefit in having multiple clocks called "functional clock" for dispc... That said, I wonder if the DISPC_FCLK, LCD1_CLK and LCD2_CLK are used at all before they are divided with the logic clock divider. If they are not, then it could make sense to speak of the logic clock as the functional clock, and call the incoming clocks something else. But for the time being, I think we should continue calling the internal fclk as core-clock. Tomi
Attachment:
signature.asc
Description: This is a digitally signed message part