Re: [PATCH 2/2] OPP/pmdomain: Fix the assignment of the required-devs

[Ulf Hansson]

On Tue, 3 Sept 2024 at 12:53, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>
> On 03-09-24, 11:54, Ulf Hansson wrote:
> > Let me try to elaborate a bit more.
> >
> > In the current code, genpd_find_opp_table() tries to find an OPP table
> > for the genpd that the device is getting attached to. Then genpd
> > passes that OPP table via devm_pm_opp_set_config(), to let the OPP
> > core to hook up a required-dev for it. This was a naive approach, as
> > that OPP table may not be the one that actually corresponds to a
> > required-opps for the required-dev. Consider the below in DT.
> >
> >         opp_table_devA: opp-table-devA {
> >                 compatible = "operating-points-v2";
> >
> >                 opp-devA-50 {
> >                         opp-hz = /bits/ 64 <2500>;
> >                         required-opps = <&opp_pd_50>; //corresponds to
> > pd_perf1's OPP table
> >                 };
> >                ....
> >
> >         devA {
> >                 compatible = "foo,bar";
> >                 power-domains = <&pd_perf0>, <&pd_perf1>; //both
> > pd_perf0 and pd_perf1 has OPP tables.
> >                 power-domain-names = "perf0", "perf1";
> >                 operating-points-v2 = <&opp_table_devA>;
> >         };
>
> I think another way forward would be to send an index along with
> required-dev information (now that you do it one by one). That index
> would be the index of the genpd in the genpd-list for the device. That
> would make it work, isn't it ?

I am not sure how that index will be much helpful, but maybe I am not
fully understanding what you propose.

Please note that the index of the power-domain doesn't need to match
the index of the required-opps.

It's only the phandle of the required-opps, at some index, that can
point out to which power-domain (and thus what required-dev) it
belongs to.

>
> I would like to avoid (another) callback from the OPP core, we already
> have few of them and I don't like them a lot. Moreover, genpd should
> be able to get the right required opp, with an index. Unless I am
> mistaken and this still doesn't solve it :)

Sorry, but I couldn't figure out a better option.

>
> > To make sure we assign the correct required-dev for cases like the
> > above, we need to let the OPP core to iterate through the available
> > required-opps and see if some of them are corresponding to the OPP
> > table for the genpd the required-dev belongs too.
> >
> > To manage this in a non-genpd specific way, I added another callback
> > in struct dev_pm_opp_config. In this way, it should work for any
> > future possible required-devs types too, I think.
>
> --
> viresh

Kind regards
Uffe