Linux clock framework development
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Stephen Boyd <sboyd@kernel.org>
Cc: Alex Elder <elder@linaro.org>,
	David Dai <daidavid1@codeaurora.org>,
	linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-kernel@vger.kernel.org, georgi.djakov@linaro.org,
	evgreen@google.com, tdas@codeaurora.org
Subject: Re: [RFC PATCH] clk: qcom: clk-rpmh: Add IPA clock support
Date: Wed, 5 Dec 2018 23:33:04 -0800	[thread overview]
Message-ID: <20181206073304.GB5232@tuxbook-pro> (raw)
In-Reply-To: <154399414865.88331.2447825064224349951@swboyd.mtv.corp.google.com>

On Tue 04 Dec 23:15 PST 2018, Stephen Boyd wrote:

> Quoting David Dai (2018-12-04 17:14:10)
> > 
> > On 12/4/2018 2:34 PM, Stephen Boyd wrote:
> > > Quoting Alex Elder (2018-12-04 13:41:47)
> > >> On 12/4/18 1:24 PM, Stephen Boyd wrote:
> > >>> Quoting David Dai (2018-12-03 19:50:13)
> > >>>> Add IPA clock support by extending the current clk rpmh driver to support
> > >>>> clocks that are managed by a different type of RPMh resource known as
> > >>>> Bus Clock Manager(BCM).
> > >>> Yes, but why? Does the IPA driver need to set clk rates and that somehow
> > >>> doesn't work as a bandwidth request?
> > >> The IPA core clock is a *clock*, not a bus.  Representing it as if
> > >> it were a bus, abusing the interconnect interface--pretending a bandwidth
> > >> request is really a clock rate request--is kind of kludgy.  I think Bjorn
> > >> and David (and maybe Georgi? I don't know) decided a long time ago that
> > >> exposing this as a clock is the right way to do it.  I agree with that.
> > >>
> > > But then we translate that clock rate into a bandwidth request to the
> > > BCM hardware? Seems really weird because it's doing the opposite of what
> > > you say is abusive. What does the IPA driver plan to do with this clk?
> > > Calculate a frequency by knowing that it really boils down to some
> > > bandwidth that then gets converted back into some clock frequency? Do we
> > > have the user somewhere that can be pointed to?
> > The clock rate is translated into a unitless threshold value sent as 
> > part of the rpmh msg
> > that BCM takes to select a performance. In this case, the unit 
> > conversion is based on
> > the unit value read from the aux data which is in Khz. I understand that 
> > this wasn't
> > explicitly mentioned anywhere and I'll improve on that next patch. 
> 
> How is this different from bus bandwidth requests? In those cases the
> bandwidth is calculated in bits per second or something like that, and
> written to the hardware so it can convert that bandwidth into kHz and
> set a bus clk frequency in the clock controller? So in the IPA case
> we've skipped the bps to kHz conversion step and gone straight to the
> clk frequency setting part? Is a BCM able to aggregate units of
> bandwidth or kHz depending on how it's configured and this BCM is
> configured for kHz?
> 

My objection to the use of msm_bus vs clock framework is not related to
how the actual interface of configuring the hardware looks like. It's
simply a matter of how this is represented in software, between some
provider and the IPA driver.

The IPA driver wants the IPA block to tick at 75MHz and I do not think
it's appropriate to achieve that by requesting a path between IPA Core
and IPA Core of 75000000 Kbytes/s.

Regards,
Bjorn

  parent reply	other threads:[~2018-12-06  7:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-04  3:50 [RFC PATCH] Add IPA clock support for clk-rpmh David Dai
2018-12-04  3:50 ` [RFC PATCH] clk: qcom: clk-rpmh: Add IPA clock support David Dai
2018-12-04 19:24   ` Stephen Boyd
2018-12-04 21:41     ` Alex Elder
2018-12-04 22:34       ` Stephen Boyd
2018-12-05  1:14         ` David Dai
2018-12-05  7:15           ` Stephen Boyd
2018-12-06  1:24             ` David Dai
2018-12-06 18:02               ` Stephen Boyd
2018-12-06  7:33             ` Bjorn Andersson [this message]
2018-12-05  2:01     ` David Dai
2018-12-04 19:24 ` [RFC PATCH] Add IPA clock support for clk-rpmh 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=20181206073304.GB5232@tuxbook-pro \
    --to=bjorn.andersson@linaro.org \
    --cc=daidavid1@codeaurora.org \
    --cc=elder@linaro.org \
    --cc=evgreen@google.com \
    --cc=georgi.djakov@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sboyd@kernel.org \
    --cc=tdas@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox