From: Rhyland Klein <rklein@nvidia.com>
To: "Emilio López" <emilio@elopez.com.ar>,
"Michael Turquette" <mturquette@baylibre.com>,
"Stephen Boyd" <sboyd@codeaurora.org>
Cc: <linux-clk@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] clk: clk_register: Correctly initialize enable_count
Date: Wed, 10 Feb 2016 13:34:16 -0500 [thread overview]
Message-ID: <56BB82A8.8010701@nvidia.com> (raw)
In-Reply-To: <56BAA6D9.9090009@elopez.com.ar>
On 2/9/2016 9:56 PM, Emilio L=F3pez wrote:
> Hi,
>=20
> El 09/02/16 a las 19:48, Rhyland Klein escribi=F3:
>> When clocks are registered, they could be enabled already in
>> hardware. As of now, the enable count will start at 0. When this
>> happens, it means a clock is enabled and the framework doesn't know
>> that, so it will always report it as disabled.
>=20
> Keep in mind that during the boot process, towards the end, unused
> clocks get disabled, so the state remains in sync. If suddenly the
> enable_count on unused clocks is not 0, this will break and unused
> clocks will remain on, wasting power.
Hmm, I had misread the logic in clk_disable_unused_subtree(), namely I
inverted the check on enable_count when I was looking at it. It does
seem like it would take care of the clocks I was referring to.
>=20
> http://lxr.free-electrons.com/source/drivers/clk/clk.c#L244
>=20
> What issue were you having that prompted you to write this patch?
I ran into the situation where a peripheral clock was enabled before the
kernel loaded, and I was trying to disable it in the clk-tegra210
driver. Calling clk_disable() won't work, as the clock doesn't have an
enable_count.
I do think that the clk_disable_unused_subtree() should pick up the
slack, but I was trying to disable it before the cleanup code to remove
unused clocks ran.
I think you are right and the clean up code should cover the situation I
was looking at.
Thanks.
-rhyland
--=20
nvpublic
WARNING: multiple messages have this Message-ID (diff)
From: Rhyland Klein <rklein@nvidia.com>
To: "Emilio López" <emilio@elopez.com.ar>,
"Michael Turquette" <mturquette@baylibre.com>,
"Stephen Boyd" <sboyd@codeaurora.org>
Cc: <linux-clk@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] clk: clk_register: Correctly initialize enable_count
Date: Wed, 10 Feb 2016 13:34:16 -0500 [thread overview]
Message-ID: <56BB82A8.8010701@nvidia.com> (raw)
In-Reply-To: <56BAA6D9.9090009@elopez.com.ar>
On 2/9/2016 9:56 PM, Emilio López wrote:
> Hi,
>
> El 09/02/16 a las 19:48, Rhyland Klein escribió:
>> When clocks are registered, they could be enabled already in
>> hardware. As of now, the enable count will start at 0. When this
>> happens, it means a clock is enabled and the framework doesn't know
>> that, so it will always report it as disabled.
>
> Keep in mind that during the boot process, towards the end, unused
> clocks get disabled, so the state remains in sync. If suddenly the
> enable_count on unused clocks is not 0, this will break and unused
> clocks will remain on, wasting power.
Hmm, I had misread the logic in clk_disable_unused_subtree(), namely I
inverted the check on enable_count when I was looking at it. It does
seem like it would take care of the clocks I was referring to.
>
> http://lxr.free-electrons.com/source/drivers/clk/clk.c#L244
>
> What issue were you having that prompted you to write this patch?
I ran into the situation where a peripheral clock was enabled before the
kernel loaded, and I was trying to disable it in the clk-tegra210
driver. Calling clk_disable() won't work, as the clock doesn't have an
enable_count.
I do think that the clk_disable_unused_subtree() should pick up the
slack, but I was trying to disable it before the cleanup code to remove
unused clocks ran.
I think you are right and the clean up code should cover the situation I
was looking at.
Thanks.
-rhyland
--
nvpublic
next prev parent reply other threads:[~2016-02-10 18:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-09 22:48 [PATCH] clk: clk_register: Correctly initialize enable_count Rhyland Klein
2016-02-10 2:56 ` Emilio López
2016-02-10 18:34 ` Rhyland Klein [this message]
2016-02-10 18:34 ` Rhyland Klein
2016-02-11 21:32 ` Michael Turquette
2016-02-11 21:32 ` Michael Turquette
2016-02-11 21:58 ` Rhyland Klein
2016-02-11 21:58 ` Rhyland Klein
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=56BB82A8.8010701@nvidia.com \
--to=rklein@nvidia.com \
--cc=emilio@elopez.com.ar \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=sboyd@codeaurora.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.