From: mturquette@ti.com (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] clk: add extension API
Date: Thu, 31 May 2012 12:09:46 -0700 [thread overview]
Message-ID: <20120531190946.GA16878@gmail.com> (raw)
In-Reply-To: <4FC68304.9050208@codeaurora.org>
On 20120530-13:28, Stephen Boyd wrote:
> On 05/30/12 12:40, Mike Turquette wrote:
> > I also wonder if exposing some of these knobs should be done in the
> > basic clock types. Meaning that instead of having additional calls in
> > the clk.h API those calls could be exposed by the basic clock types that
> > map to the actions.
>
> What do you mean by basic clock types that map to actions? Perhaps an
> example?
>
No exmaples to give, just tossing out ideas.
> >
> > The question that needs to be answered is this: do generic drivers need
> > access to these additional functions (clk.h) or just the platform code
> > which implements some of the clock logic (basic clock types &
> > platform-speciic clock types).
>
> At least for tegra it looks like they need reset assertion and
> deassertion in generic drivers.
>
> $ git grep tegra_periph_reset_assert
> arch/arm/mach-tegra/clock.c:void tegra_periph_reset_assert(struct clk *c)
> arch/arm/mach-tegra/clock.c:EXPORT_SYMBOL(tegra_periph_reset_assert);
> arch/arm/mach-tegra/include/mach/clk.h:void tegra_periph_reset_assert(struct clk *c);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.pcie_xclk);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.afi_clk);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.pex_clk);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.pcie_xclk);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.afi_clk);
> arch/arm/mach-tegra/powergate.c: tegra_periph_reset_assert(clk);
> drivers/i2c/busses/i2c-tegra.c: tegra_periph_reset_assert(i2c_dev->clk);
> drivers/input/keyboard/tegra-kbc.c: tegra_periph_reset_assert(kbc->clk);
> drivers/staging/nvec/nvec.c: tegra_periph_reset_assert(nvec->i2c_clk);
>
Ok, this is good to know. The same sort of thing is achieved via
runtime pm and the hwmod framework in OMAP code. I had given similar
feedback to Andrew Lunn for using clk_prepare/clk_unprepare to power
down the PHY for some of his IP blocks. I don't think that the clock
framework should be used for that and this clk_reset(...) stuff seems
similar.
Like Benoit, I am partial to associating these actions to module-level
APIs, not necessarily clock-level APIs.
A yardstick to determine whether or not the clock framework is the right
place for a _reset() function might be whether or not it will change the
values of struct clk's members. If we had a clk_reset(...) call it
would clearly bang some bits in a register via clk->ops->reset ... but
what data would it change in struct clk? Adjust the rate to 0? Reset
prepare_count and enable_count to 0?
If it doesn't actually change any of the bookkeeping or accounting in
the clock framework then it might be a clue that the clock framework
isn't the best place for this API.
Thanks,
Mike
WARNING: multiple messages have this Message-ID (diff)
From: Mike Turquette <mturquette@ti.com>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>,
Russell King <linux@arm.linux.org.uk>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH] clk: add extension API
Date: Thu, 31 May 2012 12:09:46 -0700 [thread overview]
Message-ID: <20120531190946.GA16878@gmail.com> (raw)
In-Reply-To: <4FC68304.9050208@codeaurora.org>
On 20120530-13:28, Stephen Boyd wrote:
> On 05/30/12 12:40, Mike Turquette wrote:
> > I also wonder if exposing some of these knobs should be done in the
> > basic clock types. Meaning that instead of having additional calls in
> > the clk.h API those calls could be exposed by the basic clock types that
> > map to the actions.
>
> What do you mean by basic clock types that map to actions? Perhaps an
> example?
>
No exmaples to give, just tossing out ideas.
> >
> > The question that needs to be answered is this: do generic drivers need
> > access to these additional functions (clk.h) or just the platform code
> > which implements some of the clock logic (basic clock types &
> > platform-speciic clock types).
>
> At least for tegra it looks like they need reset assertion and
> deassertion in generic drivers.
>
> $ git grep tegra_periph_reset_assert
> arch/arm/mach-tegra/clock.c:void tegra_periph_reset_assert(struct clk *c)
> arch/arm/mach-tegra/clock.c:EXPORT_SYMBOL(tegra_periph_reset_assert);
> arch/arm/mach-tegra/include/mach/clk.h:void tegra_periph_reset_assert(struct clk *c);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.pcie_xclk);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.afi_clk);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.pex_clk);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.pcie_xclk);
> arch/arm/mach-tegra/pcie.c: tegra_periph_reset_assert(tegra_pcie.afi_clk);
> arch/arm/mach-tegra/powergate.c: tegra_periph_reset_assert(clk);
> drivers/i2c/busses/i2c-tegra.c: tegra_periph_reset_assert(i2c_dev->clk);
> drivers/input/keyboard/tegra-kbc.c: tegra_periph_reset_assert(kbc->clk);
> drivers/staging/nvec/nvec.c: tegra_periph_reset_assert(nvec->i2c_clk);
>
Ok, this is good to know. The same sort of thing is achieved via
runtime pm and the hwmod framework in OMAP code. I had given similar
feedback to Andrew Lunn for using clk_prepare/clk_unprepare to power
down the PHY for some of his IP blocks. I don't think that the clock
framework should be used for that and this clk_reset(...) stuff seems
similar.
Like Benoit, I am partial to associating these actions to module-level
APIs, not necessarily clock-level APIs.
A yardstick to determine whether or not the clock framework is the right
place for a _reset() function might be whether or not it will change the
values of struct clk's members. If we had a clk_reset(...) call it
would clearly bang some bits in a register via clk->ops->reset ... but
what data would it change in struct clk? Adjust the rate to 0? Reset
prepare_count and enable_count to 0?
If it doesn't actually change any of the bookkeeping or accounting in
the clock framework then it might be a clue that the clock framework
isn't the best place for this API.
Thanks,
Mike
next prev parent reply other threads:[~2012-05-31 19:09 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-29 9:58 [RFC PATCH] clk: add extension API Peter De Schrijver
2012-05-29 9:58 ` Peter De Schrijver
2012-05-30 8:52 ` Stephen Boyd
2012-05-30 8:52 ` Stephen Boyd
2012-05-30 19:40 ` Mike Turquette
2012-05-30 19:40 ` Mike Turquette
2012-05-30 20:28 ` Stephen Boyd
2012-05-30 20:28 ` Stephen Boyd
2012-05-31 19:09 ` Mike Turquette [this message]
2012-05-31 19:09 ` Mike Turquette
2012-05-31 3:29 ` Saravana Kannan
2012-05-31 3:29 ` Saravana Kannan
2012-05-31 8:23 ` Peter De Schrijver
2012-05-31 8:23 ` Peter De Schrijver
2012-05-31 21:12 ` Saravana Kannan
2012-05-31 21:12 ` Saravana Kannan
2012-05-31 7:51 ` Peter De Schrijver
2012-05-31 7:51 ` Peter De Schrijver
2012-05-31 8:18 ` Felipe Balbi
2012-05-31 8:18 ` Felipe Balbi
2012-05-31 8:31 ` Peter De Schrijver
2012-05-31 8:31 ` Peter De Schrijver
2012-05-31 8:54 ` Felipe Balbi
2012-05-31 8:54 ` Felipe Balbi
2012-05-31 9:05 ` Peter De Schrijver
2012-05-31 9:05 ` Peter De Schrijver
2012-05-31 9:26 ` Felipe Balbi
2012-05-31 9:26 ` Felipe Balbi
2012-05-31 9:43 ` Peter De Schrijver
2012-05-31 9:43 ` Peter De Schrijver
2012-05-31 9:46 ` Felipe Balbi
2012-05-31 9:46 ` Felipe Balbi
2012-05-31 9:57 ` Peter De Schrijver
2012-05-31 9:57 ` Peter De Schrijver
2012-05-31 10:01 ` Felipe Balbi
2012-05-31 10:01 ` Felipe Balbi
2012-05-31 12:50 ` Cousson, Benoit
2012-05-31 12:50 ` Cousson, Benoit
2012-05-31 13:04 ` Mark Brown
2012-05-31 13:04 ` Mark Brown
2012-05-31 13:07 ` Cousson, Benoit
2012-05-31 13:07 ` Cousson, Benoit
2012-05-31 13:11 ` Mark Brown
2012-05-31 13:11 ` Mark Brown
2012-05-31 12:00 ` Mark Brown
2012-05-31 12:00 ` Mark Brown
2012-05-31 9:04 ` Stephen Boyd
2012-05-31 9:04 ` Stephen Boyd
2012-05-31 9:10 ` Peter De Schrijver
2012-05-31 9:10 ` Peter De Schrijver
2012-05-30 9:28 ` Rajendra Nayak
2012-05-30 9:28 ` Rajendra Nayak
2012-05-31 19:19 ` Mike Turquette
2012-05-31 19:19 ` Mike Turquette
2012-06-06 12:07 ` Peter De Schrijver
2012-06-06 12:07 ` Peter De Schrijver
2012-05-31 16:42 ` Stephen Warren
2012-05-31 16:42 ` Stephen Warren
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=20120531190946.GA16878@gmail.com \
--to=mturquette@ti.com \
--cc=linux-arm-kernel@lists.infradead.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.