On Tue, Jul 23, 2019 at 07:26:35AM +0530, Viresh Kumar wrote: > On 19-07-19, 17:45, Niklas Cassel wrote: > > Hello Viresh, > > > > Could you please have a look at the last two patches here: > > https://git.linaro.org/people/niklas.cassel/kernel.git/log/?h=cpr-opp-hz > > There is no sane way of providing review comments with a link to the > git tree :) > > I still had a look and I see that you don't search for max frequency > but just any OPP that has required-opps set to the level u want. Also, > can't there be multiple phandles in required-opps in your case ? For each OPP in the CPR OPP table, we need three things, opp-level, qcom,fuse-level and opp-hz. The first two can simply be parsed from the OPP node itself while iterating the CPR OPP table. The opp-hz has to be fetched from the CPU OPP table. Several OPPs might have the same qcom,fuse-level value. However, they will have unique opp-level values and unique opp-hz values. Each opp-level has a matching opp-hz. required-opps is basically a connection between a opp-hz (CPU OPP table) and and a opp-level (CPR OPP table). So there will be only one match. No need to search for max frequency. I think you are confusing this with something else. The CPR hardware has to be programmed with the highest frequency for each qcom,fuse-corner. This is done here: https://git.linaro.org/people/niklas.cassel/kernel.git/tree/drivers/power/avs/qcom-cpr.c?h=cpr-full#n1219 by saving the highest frequency for each fuse level while iterating the OPP table. There can be only one phandle in the required-opps in my case, this is one of the reasons why I prefer implementing it in the CPR driver. If it were to be implemented in OPP core, it probably has to handle multiple phandles. > > > If you like my proposal then I could send out the first patch (the one to > > OPP core) as a real patch (with an improved commit message), and > > incorporate the second patch into my CPR patch series when I send out a V2. > > Send them both in your series only, otherwise the first one is useless > anyway. Ok, will do. Kind regards, Niklas
