Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [RFC PATCH] clk: qcom: clk-rpmh: Add IPA clock support | From | David Dai <> | Date | Tue, 4 Dec 2018 17:14:10 -0800 |
| |
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
| ![\](/images/icornerr.gif) |