All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
To: "Mahapatra, Chandrabhanu" <cmahapatra@ti.com>
Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org
Subject: Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage
Date: Mon, 23 Apr 2012 06:19:13 +0000	[thread overview]
Message-ID: <1335161953.1668.11.camel@lappy> (raw)
In-Reply-To: <CAF0AtAu6xhQmQZLbW9keTMjcSfPwsVUoWa7e4fsSpHXms+pVSQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1922 bytes --]

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@ti.com> 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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
To: "Mahapatra, Chandrabhanu" <cmahapatra@ti.com>
Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org
Subject: Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage
Date: Mon, 23 Apr 2012 09:19:13 +0300	[thread overview]
Message-ID: <1335161953.1668.11.camel@lappy> (raw)
In-Reply-To: <CAF0AtAu6xhQmQZLbW9keTMjcSfPwsVUoWa7e4fsSpHXms+pVSQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1922 bytes --]

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@ti.com> 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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2012-04-23  6:19 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-02 15:13 [PATCH V3 0/3] OMAPDSS: DISPC: Enable predecimation for DMA and VRFB Chandrabhanu Mahapatra
2012-04-02 15:25 ` Chandrabhanu Mahapatra
2012-04-02 15:13 ` [PATCH V3 1/3] OMAPDSS: DISPC: Enable predecimation Chandrabhanu Mahapatra
2012-04-02 15:25   ` Chandrabhanu Mahapatra
2012-04-02 15:13   ` [PATCH V3 2/3] OMAPDSS: DISPC: Handle synclost errors in OMAP3 Chandrabhanu Mahapatra
2012-04-02 15:25     ` Chandrabhanu Mahapatra
2012-04-02 15:13     ` [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage Chandrabhanu Mahapatra
2012-04-02 15:25       ` Chandrabhanu Mahapatra
2012-04-19 13:11       ` Tomi Valkeinen
2012-04-19 13:11         ` Tomi Valkeinen
2012-04-20  6:30         ` Mahapatra, Chandrabhanu
2012-04-20  6:42           ` Mahapatra, Chandrabhanu
2012-04-20 13:31       ` Chandrabhanu Mahapatra
2012-04-20 13:43         ` Chandrabhanu Mahapatra
2012-04-20 14:29         ` Tomi Valkeinen
2012-04-20 14:29           ` Tomi Valkeinen
2012-04-23  4:35           ` Mahapatra, Chandrabhanu
2012-04-23  4:47             ` Mahapatra, Chandrabhanu
2012-04-23  6:19             ` Tomi Valkeinen [this message]
2012-04-23  6:19               ` Tomi Valkeinen
2012-04-23  6:46       ` Chandrabhanu Mahapatra
2012-04-23  6:58         ` Chandrabhanu Mahapatra
2012-04-23  7:50         ` Tomi Valkeinen
2012-04-23  7:50           ` Tomi Valkeinen
     [not found] <[PATCH V2 0/3] OMAPDSS: DISPC: Enable predecimation for DMA and VRFB>
2012-04-02 13:59 ` [PATCH V3 0/3] OMAPDSS: DISPC: Enable predecimation for DMA and VRFB Chandrabhanu Mahapatra
2012-04-02 14:11   ` Chandrabhanu Mahapatra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1335161953.1668.11.camel@lappy \
    --to=tomi.valkeinen@ti.com \
    --cc=cmahapatra@ti.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.