From: Jerome Brunet <jbrunet@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
linux-amlogic@lists.infradead.org, narmstrong@baylibre.com
Cc: mturquette@baylibre.com, linux-clk@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/1] clk: Meson8/8b/8m2: fix the mali clock flags
Date: Thu, 26 Dec 2019 10:06:25 +0100 [thread overview]
Message-ID: <1jimm3pib2.fsf@starbuckisacylon.baylibre.com> (raw)
In-Reply-To: <20191224033636.1BB3F206B7@mail.kernel.org>
On Tue 24 Dec 2019 at 04:36, Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Jerome Brunet (2019-12-16 11:17:21)
>>
>> On Mon 16 Dec 2019 at 18:50, Stephen Boyd <sboyd@kernel.org> wrote:
>>
>> > Quoting Jerome Brunet (2019-12-16 01:13:31)
>> >>
>> >> *updated last* which crucial to your use case.
>> >>
>> >> I just wonder if this crucial part something CCF guarantee and you can
>> >> rely on it ... or if it might break in the future.
>> >>
>> >> Stephen, any thoughts on this ?
>> >
>> > We have problems with the order in which we call the set_rate clk_op.
>> > Sometimes clk providers want us to call from leaf to root but instead we
>> > call from root to leaf because of implementation reasons. Controlling
>> > the order in which clk operations are done is an unsolved problem. But
>> > yes, in the future I'd like to see us introduce the vaporware that is
>> > coordinated clk rates that would allow clk providers to decide what this
>> > order should be, instead of having to do this "root-to-leaf" update.
>> > Doing so would help us with the clk dividers that have some parent
>> > changing rate that causes the downstream device to be overclocked while
>> > we change the parent before the divider.
>> >
>> > If there are more assumptions like this about how the CCF is implemented
>> > then we'll have to be extra careful to not disturb the "normal" order of
>> > operations when introducing something that allows clk providers to
>> > modify it.
>>
>> I understand that CCR would, in theory, allow to define that sort of
>> details. Still defining (and documenting) the default behavior would be
>> nice.
>>
>> So the question is:
>> * Can we rely set_rate() doing a root-to-leaf update until CCR comes
>> around ?
>> * If not, for use cases like the one described by Martin, I guess we
>> are stuck with the notifier ? Or would you have something else to
>> propose ?
>
> I suppose we should just state that clk_set_rate() should do a
> root-to-leaf update. It's not like anyone is interested in changing
> this behavior. The notifier is not ideal. I've wanted to add a new
> clk_op that would cover some amount of the notifier users by having a
> 'pre_set_rate' clk op that can mux the clk over to something safe or
> setup a divider to something that is known to be safe and work. Then we
> can avoid having to register for a notifier just to do something right
> before the root-to-leaf update happens.
>
Martin,
It looks like a green light to me ;) Just add a detailed comment on the
mali top clock explaining things and it should be alright.
>>
>> >
>> > Also, isn't CLK_SET_RATE_GATE broken in the case that clk_set_rate()
>> > isn't called on that particular clk? I seem to recall that the flag only
>> > matters when it's applied to the "leaf" or entry point into the CCF from
>> > a consumer API.
>>
>> It did but not anymore
>>
>> > I've wanted to fix that but never gotten around to it.
>>
>> I fixed that already :P
>> CLK_SET_RATE_GATE is a special case of clock protect. The clock is
>> protecting itself so it is going down through the tree.
>>
>
> Ahaha ok. As you can see I'm trying to forget clock protect ;-)
>
>
>>
>> > The whole flag sort of irks me because I don't understand what consumers
>> > are supposed to do when this flag is set on a clk. How do they discover
>> > it?
>>
>> Actually (ATM) the consumer is not even aware of it. If a clock with
>> CLK_SET_RATE_GATE is enabled, it will return the current rate to
>> .round_rate() and .set_rate() ... as if it was fixed.
>
> And then when the clk is disabled it will magically "unstick" and start
> to accept the same rate request again?
>
Exactly
>>
>> > They're supposed to "just know" and turn off the clk first and then
>> > call clk_set_rate()?
>>
>> ATM, yes ... if CCF cannot switch to another "unlocked" subtree (the
>> case here)
>>
>> > Why can't the framework do this all in the clk_set_rate() call?
>>
>> When there is multiple consumers the behavior would become a bit
>> difficult to predict and drivers may have troubles anticipating that,
>> maybe, the clock is locked.
>
> Fun times!
prev parent reply other threads:[~2019-12-26 9:06 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-15 21:01 [PATCH 0/1] clk: Meson8/8b/8m2: fix the mali clock flags Martin Blumenstingl
2019-12-15 21:01 ` [PATCH 1/1] clk: meson: meson8b: make the CCF use the glitch-free "mali" mux Martin Blumenstingl
2019-12-16 9:13 ` [PATCH 0/1] clk: Meson8/8b/8m2: fix the mali clock flags Jerome Brunet
2019-12-16 17:50 ` Stephen Boyd
2019-12-16 19:17 ` Jerome Brunet
2019-12-24 3:36 ` Stephen Boyd
2019-12-26 9:06 ` Jerome Brunet [this message]
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=1jimm3pib2.fsf@starbuckisacylon.baylibre.com \
--to=jbrunet@baylibre.com \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.blumenstingl@googlemail.com \
--cc=mturquette@baylibre.com \
--cc=narmstrong@baylibre.com \
--cc=sboyd@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).