linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Stephan Gerhold <stephan@gerhold.net>
Cc: Shawn Guo <shawn.guo@linaro.org>, Stephen Boyd <sboyd@kernel.org>,
	Loic Poulain <loic.poulain@linaro.org>,
	linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/3] clk: qcom: smd-rpm: Report enable state to framework
Date: Tue, 9 Nov 2021 07:56:27 -0800	[thread overview]
Message-ID: <YYqaK/Evzuup0ocs@ripper> (raw)
In-Reply-To: <YYpMzau3CWRQYlkJ@gerhold.net>

On Tue 09 Nov 02:26 PST 2021, Stephan Gerhold wrote:

> Hi Shawn,
> 
> On Tue, Nov 09, 2021 at 10:25:55AM +0800, Shawn Guo wrote:
> > Currently the enable state of smd-rpm clocks are not properly reported
> > back to framework due to missing .is_enabled and .is_prepared hooks.
> > This causes a couple of issues.
> > 
> > - All those unused clocks are not voted for off, because framework has
> >   no knowledge that they are unused.  It becomes a problem for vlow
> >   power mode support, as we do not have every single RPM clock claimed
> >   and voted for off by client devices, and rely on clock framework to
> >   disable those unused RPM clocks.
> > 
> 
> I posted a similar patch a bit more than a year ago [1]. Back then one
> of the concerns was that we might disable critical clocks just because
> they have no driver using it actively. For example, not all of the
> platforms using clk-smd-rpm already have an interconnect driver.
> Disabling the interconnect related clocks will almost certainly make the
> device lock up completely. (I tried it back then, it definitely does...)
> 
> I proposed adding CLK_IGNORE_UNUSED for the interconnect related clocks
> back then [2] which would allow disabling most of the clocks at least.
> Stephen Boyd had an alternative proposal to instead move the
> interconnect related clocks completely out of clk-smd-rpm [3].
> But I'm still unsure how this would work in a backwards compatible way. [4]
> 

With the introduction of QoS the interconnect drivers need to be mmio
devices, and plural, while in order to talk to the RPM we need something
on the rpmsg bus.

So I don't think Stephen's proposal will work, unless we like in the
RPMh case comes up with an equivalent of the bcm-voter (which just moved
the clocks from one clock driver to a "clock" driver).


On the other hand, if clocks and the clk-smd-rpm driver in particular
moves to sync_state then this wouldn't be a problem...

Regards,
Bjorn

> Since your patches are more or less identical I'm afraid the same
> concerns still need to be solved somehow. :)
> 
> Thanks,
> Stephan
> 
> [1]: https://lore.kernel.org/linux-arm-msm/20200817140908.185976-1-stephan@gerhold.net/
> [2]: https://lore.kernel.org/linux-arm-msm/20200818080738.GA46574@gerhold.net/
> [3]: https://lore.kernel.org/linux-arm-msm/159796605593.334488.8355244657387381953@swboyd.mtv.corp.google.com/
> [4]: https://lore.kernel.org/linux-arm-msm/20200821064857.GA905@gerhold.net/

  reply	other threads:[~2021-11-09 15:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-09  2:25 [PATCH 0/3] clk: qcom: smd-rpm: Report enable state to framework Shawn Guo
2021-11-09  2:25 ` [PATCH 1/3] clk: qcom: smd-rpm: Mark clock enabled in clk_smd_rpm_handoff() Shawn Guo
2021-11-09  2:25 ` [PATCH 2/3] clk: qcom: smd-rpm: Add .is_enabled hook Shawn Guo
2021-11-09  2:25 ` [PATCH 3/3] clk: qcom: smd-rpm: Add .is_prepared hook Shawn Guo
2021-11-09 10:26 ` [PATCH 0/3] clk: qcom: smd-rpm: Report enable state to framework Stephan Gerhold
2021-11-09 15:56   ` Bjorn Andersson [this message]
2021-11-10 13:15   ` Shawn Guo
2021-11-10 13:48     ` Bjorn Andersson
2021-11-11  9:39       ` Shawn Guo
2021-12-06 15:42         ` Bjorn Andersson
2021-11-10 14:09     ` Stephan Gerhold
2021-11-10 14:58       ` Bjorn Andersson
2021-12-09  9:10   ` Stephen Boyd

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=YYqaK/Evzuup0ocs@ripper \
    --to=bjorn.andersson@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=loic.poulain@linaro.org \
    --cc=sboyd@kernel.org \
    --cc=shawn.guo@linaro.org \
    --cc=stephan@gerhold.net \
    /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).