From: Felipe Balbi <balbi@ti.com>
To: Govindraj <govindraj.ti@gmail.com>
Cc: balbi@ti.com, "Govindraj.R" <govindraj.raja@ti.com>,
linux-omap@vger.kernel.org, linux-serial@vger.kernel.org,
linux-pm@lists.linux-foundation.org,
"Rafael J. Wysocki" <rjw@sisk.pl>, Paul Walmsley <paul@pwsan.com>,
Kevin Hilman <khilman@ti.com>,
Vishwanath Sripathy <vishwanath.bs@ti.com>,
Partha Basak <p-basak2@ti.com>,
Santosh Shilimkar <santosh.shilimkar@ti.com>,
Rajendra Nayak <rnayak@ti.com>, Benoit Cousson <b-cousson@ti.com>,
Tero Kristo <t-kristo@ti.com>
Subject: Re: [RFC v2]: Issues implementing clock handling mechanism within UART driver
Date: Mon, 1 Aug 2011 12:03:30 +0300 [thread overview]
Message-ID: <20110801090328.GG31013@legolas.emea.dhcp.ti.com> (raw)
In-Reply-To: <CAAL8m4wdKu_XGFaGxuAJ9uypRyyQMkR8HL7Y6E=pBkwX=L43nA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8335 bytes --]
Hi,
On Fri, Jul 29, 2011 at 08:43:49PM +0530, Govindraj wrote:
[giant snip]
> Actually there is much more than this:
>
> <<SNIP>>
>
> diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
> index 180299e..221ffb9 100644
> --- a/arch/arm/mach-omap2/clock.c
> +++ b/arch/arm/mach-omap2/clock.c
> @@ -12,7 +12,8 @@
> * it under the terms of the GNU General Public License version 2 as
> * published by the Free Software Foundation.
> */
> -#undef DEBUG
> +//#undef DEBUG
> +#define DEBUG
trailing... but you know that :-p
> @@ -254,14 +255,14 @@ void omap2_clk_disable(struct clk *clk)
> return;
> }
>
> - pr_debug("clock: %s: decrementing usecount\n", clk->name);
> +// pr_debug("clock: %s: decrementing usecount\n", clk->name);
>
> clk->usecount--;
>
> if (clk->usecount > 0)
> return;
>
> - pr_debug("clock: %s: disabling in hardware\n", clk->name);
> +// pr_debug("clock: %s: disabling in hardware\n", clk->name);
>
> if (clk->ops && clk->ops->disable) {
> trace_clock_disable(clk->name, 0, smp_processor_id());
this hunk is unnecessary. Clocks are always on when they are called.
> @@ -290,14 +291,14 @@ int omap2_clk_enable(struct clk *clk)
> {
> int ret;
>
> - pr_debug("clock: %s: incrementing usecount\n", clk->name);
> +// pr_debug("clock: %s: incrementing usecount\n", clk->name);
>
> clk->usecount++;
>
> if (clk->usecount > 1)
> return 0;
>
> - pr_debug("clock: %s: enabling in hardware\n", clk->name);
> +// pr_debug("clock: %s: enabling in hardware\n", clk->name);
these two is ok.
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 7ed0479..8ca7d40 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -124,7 +124,8 @@
> * XXX error return values should be checked to ensure that they are
> * appropriate
> */
> -#undef DEBUG
> +//#undef DEBUG
> +#define DEBUG
trailing.
> @@ -597,7 +598,8 @@ static int _enable_clocks(struct omap_hwmod *oh)
> {
> int i;
>
> - pr_debug("omap_hwmod: %s: enabling clocks\n", oh->name);
> + if (strcmp(oh->class->name, "uart"))
> + pr_debug("omap_hwmod: %s: enabling clocks\n", oh->name);
instead of doing checks, you could move the print to the end of the
function, when clocks are already enabled. When doind that, of course,
update the comment to say "%s: clocks enabled\n".
> @@ -627,7 +629,8 @@ static int _disable_clocks(struct omap_hwmod *oh)
> {
> int i;
>
> - pr_debug("omap_hwmod: %s: disabling clocks\n", oh->name);
> + if (strcmp(oh->class->name, "uart"))
> + pr_debug("omap_hwmod: %s: disabling clocks\n", oh->name);
check not needed, clocks are still on.
>
> if (oh->_clk)
> clk_disable(oh->_clk);
> @@ -1232,7 +1235,8 @@ static int _enable(struct omap_hwmod *oh)
> return -EINVAL;
> }
>
> - pr_debug("omap_hwmod: %s: enabling\n", oh->name);
> + if (strcmp(oh->class->name, "uart"))
> + pr_debug("omap_hwmod: %s: enabling\n", oh->name);
move it further down.
> @@ -1264,8 +1268,9 @@ static int _enable(struct omap_hwmod *oh)
> }
> } else {
> _disable_clocks(oh);
> - pr_debug("omap_hwmod: %s: _wait_target_ready: %d\n",
> - oh->name, r);
> + if (strcmp(oh->class->name, "uart"))
> + pr_debug("omap_hwmod: %s: _wait_target_ready: %d\n",
> + oh->name, r);
instead of adding check, move the print before _disable_clocks(oh).
> @@ -1287,7 +1292,8 @@ static int _idle(struct omap_hwmod *oh)
> return -EINVAL;
> }
>
> - pr_debug("omap_hwmod: %s: idling\n", oh->name);
> + if (strcmp(oh->class->name, "uart"))
> + pr_debug("omap_hwmod: %s: idling\n", oh->name);
I believe clocks are still on here too, no checks needed.
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 49fc0df..7b27704 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -75,7 +75,8 @@
> * (device must be reinitialized at this point to use it again)
> *
> */
> -#undef DEBUG
> +//#undef DEBUG
> +#define DEBUG
trailing.
> @@ -114,7 +115,8 @@ static int _omap_device_activate(struct
> omap_device *od, u8 ignore_lat)
> {
> struct timespec a, b, c;
>
> - pr_debug("omap_device: %s: activating\n", od->pdev.name);
> + if (strcmp(od->hwmods[0]->class->name, "uart"))
> + pr_debug("omap_device: %s: activating\n", od->pdev.name);
move it to the end of the function.
> @@ -138,25 +140,29 @@ static int _omap_device_activate(struct
> omap_device *od, u8 ignore_lat)
> c = timespec_sub(b, a);
> act_lat = timespec_to_ns(&c);
>
> - pr_debug("omap_device: %s: pm_lat %d: activate: elapsed time "
> - "%llu nsec\n", od->pdev.name, od->pm_lat_level,
> - act_lat);
> + if (strcmp(od->hwmods[0]->class->name, "uart"))
> + pr_debug("omap_device: %s: pm_lat %d: activate: elapsed time "
> + "%llu nsec\n", od->pdev.name, od->pm_lat_level,
> + act_lat);
move it further down.
>
> if (act_lat > odpl->activate_lat) {
> odpl->activate_lat_worst = act_lat;
> if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
> odpl->activate_lat = act_lat;
> - pr_warning("omap_device: %s.%d: new worst case "
> - "activate latency %d: %llu\n",
> - od->pdev.name, od->pdev.id,
> - od->pm_lat_level, act_lat);
> - } else
> - pr_warning("omap_device: %s.%d: activate "
> - "latency %d higher than exptected. "
> - "(%llu > %d)\n",
> - od->pdev.name, od->pdev.id,
> - od->pm_lat_level, act_lat,
> - odpl->activate_lat);
> + if (strcmp(od->hwmods[0]->class->name, "uart"))
> + pr_warning("omap_device: %s.%d: new worst case "
> + "activate latency %d: %llu\n",
> + od->pdev.name, od->pdev.id,
> + od->pm_lat_level, act_lat);
> + } else {
> + if (strcmp(od->hwmods[0]->class->name, "uart"))
> + pr_warning("omap_device: %s.%d: activate "
> + "latency %d higher than exptected. "
> + "(%llu > %d)\n",
> + od->pdev.name, od->pdev.id,
> + od->pm_lat_level, act_lat,
> + odpl->activate_lat);
->activate_func() has already been called here, clocks are already on.
> @@ -183,7 +189,8 @@ static int _omap_device_deactivate(struct
> omap_device *od, u8 ignore_lat)
> {
> struct timespec a, b, c;
>
> - pr_debug("omap_device: %s: deactivating\n", od->pdev.name);
> + if (strcmp(od->hwmods[0]->class->name, "uart"))
> + pr_debug("omap_device: %s: deactivating\n", od->pdev.name);
clocks are still on here.
> @@ -206,25 +213,29 @@ static int _omap_device_deactivate(struct
> omap_device *od, u8 ignore_lat)
> c = timespec_sub(b, a);
> deact_lat = timespec_to_ns(&c);
>
> - pr_debug("omap_device: %s: pm_lat %d: deactivate: elapsed time "
> - "%llu nsec\n", od->pdev.name, od->pm_lat_level,
> - deact_lat);
> + if (strcmp(od->hwmods[0]->class->name, "uart"))
> + pr_debug("omap_device: %s: pm_lat %d: deactivate: elapsed time "
> + "%llu nsec\n", od->pdev.name, od->pm_lat_level,
> + deact_lat);
I'll leave this to Kevin to decide what to do, but clocks are off
here...
> if (deact_lat > odpl->deactivate_lat) {
> odpl->deactivate_lat_worst = deact_lat;
> if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
> odpl->deactivate_lat = deact_lat;
> - pr_warning("omap_device: %s.%d: new worst case "
> - "deactivate latency %d: %llu\n",
> - od->pdev.name, od->pdev.id,
> - od->pm_lat_level, deact_lat);
> - } else
> - pr_warning("omap_device: %s.%d: deactivate "
> + if (strcmp(od->hwmods[0]->class->name, "uart"))
> + pr_warning("omap_device: %s.%d: new worst case "
> + "deactivate latency %d: %llu\n",
> + od->pdev.name, od->pdev.id,
> + od->pm_lat_level, deact_lat);
> + } else {
> + if (strcmp(od->hwmods[0]->class->name, "uart"))
> + pr_warning("omap_device: %s.%d: deactivate "
> "latency %d higher than exptected. "
> "(%llu > %d)\n",
> od->pdev.name, od->pdev.id,
> od->pm_lat_level, deact_lat,
> odpl->deactivate_lat);
> + }
and here...
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
next prev parent reply other threads:[~2011-08-01 9:03 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-28 9:29 [RFC v2]: Issues implementing clock handling mechanism within UART driver Govindraj.R
2011-07-29 9:55 ` Felipe Balbi
2011-07-29 9:55 ` Felipe Balbi
2011-07-29 11:24 ` Govindraj
2011-07-29 11:24 ` Govindraj
2011-07-29 11:37 ` Felipe Balbi
2011-07-29 11:59 ` Govindraj
2011-07-29 11:59 ` Govindraj
2011-07-29 12:19 ` Felipe Balbi
2011-07-29 12:19 ` Felipe Balbi
2011-07-29 12:58 ` Govindraj
2011-07-29 12:58 ` Govindraj
2011-07-29 14:02 ` Felipe Balbi
2011-07-29 15:13 ` Govindraj
2011-08-01 9:03 ` Felipe Balbi
2011-08-01 9:03 ` Felipe Balbi [this message]
2011-08-01 9:56 ` Raja, Govindraj
2011-08-01 10:02 ` Felipe Balbi
2011-08-01 12:46 ` Govindraj
2011-08-01 12:46 ` Govindraj
2011-08-01 10:02 ` Felipe Balbi
2011-08-01 10:00 ` Govindraj
2011-08-01 10:00 ` Govindraj
2011-07-29 15:13 ` Govindraj
2011-07-29 14:02 ` Felipe Balbi
2011-07-29 11:37 ` Felipe Balbi
2011-07-28 9:29 Govindraj.R
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=20110801090328.GG31013@legolas.emea.dhcp.ti.com \
--to=balbi@ti.com \
--cc=b-cousson@ti.com \
--cc=govindraj.raja@ti.com \
--cc=govindraj.ti@gmail.com \
--cc=khilman@ti.com \
--cc=linux-omap@vger.kernel.org \
--cc=linux-pm@lists.linux-foundation.org \
--cc=linux-serial@vger.kernel.org \
--cc=p-basak2@ti.com \
--cc=paul@pwsan.com \
--cc=rjw@sisk.pl \
--cc=rnayak@ti.com \
--cc=santosh.shilimkar@ti.com \
--cc=t-kristo@ti.com \
--cc=vishwanath.bs@ti.com \
/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.