Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 55F88602F4
Subject: Re: [RFC PATCH] clk: qcom: clk-rpmh: Add IPA clock support
To:     Stephen Boyd <sboyd@kernel.org>, Alex Elder <elder@linaro.org>,
        linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     georgi.djakov@linaro.org, bjorn.andersson@linaro.org,
        evgreen@google.com, tdas@codeaurora.org
References: <1543895413-1553-1-git-send-email-daidavid1@codeaurora.org>
 <1543895413-1553-2-git-send-email-daidavid1@codeaurora.org>
 <154395145491.88331.1174781210192403998@swboyd.mtv.corp.google.com>
 <8dafe631-4b16-94cd-392e-84728f2bb382@linaro.org>
 <154396284056.88331.12279283832884556349@swboyd.mtv.corp.google.com>
From:   David Dai <daidavid1@codeaurora.org>
Message-ID: <edeebce5-24a2-b844-6bff-781017949e2d@codeaurora.org>
Date:   Tue, 4 Dec 2018 17:14:10 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <154396284056.88331.12279283832884556349@swboyd.mtv.corp.google.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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. 
Here's a link to
the IPA driver implementation: https://lkml.org/lkml/2018/11/7/220

>
> Of course, none of these details are in the commit text so it's really
> hard for me as a bystander to figure this all out.  So again, please add
> these sorts of details to the commit text so we can be "sold" on the
> idea of the patch instead of stating what the patch does.
Understood, I'll be as detailed and as explicit as I can in the future.

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project