Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux