diff mbox series

[v1,2/3] OPP: Add function to look up required OPP's for a given OPP

Message ID 20190622003449.33707-3-saravanak@google.com (mailing list archive)
State Superseded, archived
Headers show
Series Add required-opps support to devfreq passive gov | expand

Commit Message

Saravana Kannan June 22, 2019, 12:34 a.m. UTC
Add a function that allows looking up required OPPs given a source OPP
table, destination OPP table and the source OPP.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/opp/core.c     | 54 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_opp.h | 11 +++++++++
 2 files changed, 65 insertions(+)

Comments

Chanwoo Choi June 22, 2019, 11:49 a.m. UTC | #1
Hi,

Absolutely, I like this approach. I think that it is necessary to make
the connection
between frequencies of devices. But, I have a question on below.

2019년 6월 22일 (토) 오전 9:35, Saravana Kannan <saravanak@google.com>님이 작성:
>
> Add a function that allows looking up required OPPs given a source OPP
> table, destination OPP table and the source OPP.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  drivers/opp/core.c     | 54 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pm_opp.h | 11 +++++++++
>  2 files changed, 65 insertions(+)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 74c7bdc6f463..4f7870bffbf8 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1830,6 +1830,60 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table,
>                 dev_err(virt_dev, "Failed to find required device entry\n");
>  }
>
> +/**
> + * dev_pm_opp_xlate_opp() - Find required OPP for src_table OPP.
> + * @src_table: OPP table which has dst_table as one of its required OPP table.
> + * @dst_table: Required OPP table of the src_table.
> + * @pstate: OPP of the src_table.
> + *
> + * This function returns the OPP (present in @dst_table) pointed out by the
> + * "required-opps" property of the OPP (present in @src_table).
> + *
> + * The callers are required to call dev_pm_opp_put() for the returned OPP after
> + * use.
> + *
> + * Return: destination table OPP on success, otherwise NULL on errors.
> + */
> +struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
> +                                       struct opp_table *dst_table,
> +                                       struct dev_pm_opp *src_opp)
> +{
> +       struct dev_pm_opp *opp, *dest_opp = NULL;
> +       int i;
> +
> +       if (!src_table || !dst_table || !src_opp)
> +               return NULL;
> +
> +       for (i = 0; i < src_table->required_opp_count; i++) {
> +               if (src_table->required_opp_tables[i]->np == dst_table->np)
> +                       break;
> +       }
> +
> +       if (unlikely(i == src_table->required_opp_count)) {
> +               pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
> +                      __func__, src_table, dst_table);
> +               return NULL;
> +       }
> +
> +       mutex_lock(&src_table->lock);
> +
> +       list_for_each_entry(opp, &src_table->opp_list, node) {
> +               if (opp == src_opp) {
> +                       dest_opp = opp->required_opps[i];

Correct me if I am wrong. This patch assume that 'i' index is same on between
[1] and [2]. But in order to guarantee this assumption, all OPP entries
in the same opp_table have to have the same number of 'required-opps' properties
and keep the sequence among 'required-opps' entries.

[1] src_table->required_opp_tables[i]->np
[2] opp->required_opps[I];

For example, three OPP entries in the 'parent_bus_opp'
have the different sequence of 'required-opps' and the different
number of 'required-opps'. Is it no problem?

parent_bus_opp: opp_table {
    compatible = "operating-points-v2";

    opp2 {
        opp-hz = /bits/ 64 <200000>;
        required-opps = <&child_bus_a_opp2>, <&child_bus_b_opp2>,
<&child_bus_c_opp2>;
    };

    opp1 {
        opp-hz = /bits/ 64 <200000>;
        // change the sequence between child_bus_b_opp2  and child_bus_c_opp2
        required-opps = <&child_bus_a_opp2>, <&child_bus_c_opp2>,
<&child_bus_b_opp2>
    };

    opp0 {
        opp-hz = /bits/ 64 <200000>;
        // missing 'child_bus_a_opp2'
        required-opps = <&child_bus_c_opp2>, <&child_bus_b_opp2>
    };

}



> +                       dev_pm_opp_get(dest_opp);
> +                       goto unlock;
> +               }
> +       }
> +
> +       pr_err("%s: Couldn't find matching OPP (%p: %p)\n", __func__, src_table,
> +              dst_table);
> +
> +unlock:
> +       mutex_unlock(&src_table->lock);
> +
> +       return dest_opp;
> +}
> +
>  /**
>   * dev_pm_opp_xlate_performance_state() - Find required OPP's pstate for src_table.
>   * @src_table: OPP table which has dst_table as one of its required OPP table.
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index b150fe97ce5a..bc5c68bdfc8d 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -134,6 +134,9 @@ void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table);
>  struct opp_table *dev_pm_opp_set_genpd_virt_dev(struct device *dev, struct device *virt_dev, int index);
>  void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table, struct device *virt_dev);
>  int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
> +struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
> +                                       struct opp_table *dst_table,
> +                                       struct dev_pm_opp *src_opp);
>  int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
>  int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
>  int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
> @@ -307,6 +310,14 @@ static inline int dev_pm_opp_xlate_performance_state(struct opp_table *src_table
>         return -ENOTSUPP;
>  }
>
> +static inline struct dev_pm_opp *dev_pm_opp_xlate_opp(
> +                                               struct opp_table *src_table,
> +                                               struct opp_table *dst_table,
> +                                               struct dev_pm_opp *src_opp)
> +{
> +       return NULL;
> +}
> +
>  static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>  {
>         return -ENOTSUPP;
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
Saravana Kannan June 22, 2019, 9:41 p.m. UTC | #2
On Sat, Jun 22, 2019 at 4:50 AM Chanwoo Choi <cwchoi00@gmail.com> wrote:
>
> Hi,
>
> Absolutely, I like this approach. I think that it is necessary to make
> the connection
> between frequencies of devices.

Happy to hear that.

> But, I have a question on below.
>
> 2019년 6월 22일 (토) 오전 9:35, Saravana Kannan <saravanak@google.com>님이 작성:
> >
> > Add a function that allows looking up required OPPs given a source OPP
> > table, destination OPP table and the source OPP.
> >
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> >  drivers/opp/core.c     | 54 ++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/pm_opp.h | 11 +++++++++
> >  2 files changed, 65 insertions(+)
> >
> > diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> > index 74c7bdc6f463..4f7870bffbf8 100644
> > --- a/drivers/opp/core.c
> > +++ b/drivers/opp/core.c
> > @@ -1830,6 +1830,60 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table,
> >                 dev_err(virt_dev, "Failed to find required device entry\n");
> >  }
> >
> > +/**
> > + * dev_pm_opp_xlate_opp() - Find required OPP for src_table OPP.
> > + * @src_table: OPP table which has dst_table as one of its required OPP table.
> > + * @dst_table: Required OPP table of the src_table.
> > + * @pstate: OPP of the src_table.
> > + *
> > + * This function returns the OPP (present in @dst_table) pointed out by the
> > + * "required-opps" property of the OPP (present in @src_table).
> > + *
> > + * The callers are required to call dev_pm_opp_put() for the returned OPP after
> > + * use.
> > + *
> > + * Return: destination table OPP on success, otherwise NULL on errors.
> > + */
> > +struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
> > +                                       struct opp_table *dst_table,
> > +                                       struct dev_pm_opp *src_opp)
> > +{
> > +       struct dev_pm_opp *opp, *dest_opp = NULL;
> > +       int i;
> > +
> > +       if (!src_table || !dst_table || !src_opp)
> > +               return NULL;
> > +
> > +       for (i = 0; i < src_table->required_opp_count; i++) {
> > +               if (src_table->required_opp_tables[i]->np == dst_table->np)
> > +                       break;
> > +       }
> > +
> > +       if (unlikely(i == src_table->required_opp_count)) {
> > +               pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
> > +                      __func__, src_table, dst_table);
> > +               return NULL;
> > +       }
> > +
> > +       mutex_lock(&src_table->lock);
> > +
> > +       list_for_each_entry(opp, &src_table->opp_list, node) {
> > +               if (opp == src_opp) {
> > +                       dest_opp = opp->required_opps[i];
>
> Correct me if I am wrong. This patch assume that 'i' index is same on between
> [1] and [2]. But in order to guarantee this assumption, all OPP entries
> in the same opp_table have to have the same number of 'required-opps' properties
> and keep the sequence among 'required-opps' entries.
>
> [1] src_table->required_opp_tables[i]->np
> [2] opp->required_opps[I];
>
> For example, three OPP entries in the 'parent_bus_opp'
> have the different sequence of 'required-opps' and the different
> number of 'required-opps'. Is it no problem?
>
> parent_bus_opp: opp_table {
>     compatible = "operating-points-v2";
>
>     opp2 {
>         opp-hz = /bits/ 64 <200000>;
>         required-opps = <&child_bus_a_opp2>, <&child_bus_b_opp2>,
> <&child_bus_c_opp2>;
>     };
>
>     opp1 {
>         opp-hz = /bits/ 64 <200000>;
>         // change the sequence between child_bus_b_opp2  and child_bus_c_opp2
>         required-opps = <&child_bus_a_opp2>, <&child_bus_c_opp2>,
> <&child_bus_b_opp2>
>     };
>
>     opp0 {
>         opp-hz = /bits/ 64 <200000>;
>         // missing 'child_bus_a_opp2'
>         required-opps = <&child_bus_c_opp2>, <&child_bus_b_opp2>
>     };
>
> }
>

I get your question. If I'm not mistaken the OPP framework DT parsing
code makes the assumption that the required-opps list has the phandles
in the same order for each "row" in the OPP table. It actually only
looks at the first OPP entry to figure out the list of required OPP
tables.

Technically one can write code to deal with random order of the
required-opp list, but doesn't seem like that's worth it because
there's no need to have that order all mixed up in DT. And even if
someone wants to add support for that, I don't think improving the DT
parsing to handle random order would be part of this patch series.

-Saravana

> --
> Best Regards,
> Chanwoo Choi
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
Chanwoo Choi June 23, 2019, 4:27 a.m. UTC | #3
Hi,

2019년 6월 23일 (일) 오전 6:42, Saravana Kannan <saravanak@google.com>님이 작성:
>
> On Sat, Jun 22, 2019 at 4:50 AM Chanwoo Choi <cwchoi00@gmail.com> wrote:
> >
> > Hi,
> >
> > Absolutely, I like this approach. I think that it is necessary to make
> > the connection
> > between frequencies of devices.
>
> Happy to hear that.
>
> > But, I have a question on below.
> >
> > 2019년 6월 22일 (토) 오전 9:35, Saravana Kannan <saravanak@google.com>님이 작성:
> > >
> > > Add a function that allows looking up required OPPs given a source OPP
> > > table, destination OPP table and the source OPP.
> > >
> > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > ---
> > >  drivers/opp/core.c     | 54 ++++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/pm_opp.h | 11 +++++++++
> > >  2 files changed, 65 insertions(+)
> > >
> > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> > > index 74c7bdc6f463..4f7870bffbf8 100644
> > > --- a/drivers/opp/core.c
> > > +++ b/drivers/opp/core.c
> > > @@ -1830,6 +1830,60 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table,
> > >                 dev_err(virt_dev, "Failed to find required device entry\n");
> > >  }
> > >
> > > +/**
> > > + * dev_pm_opp_xlate_opp() - Find required OPP for src_table OPP.
> > > + * @src_table: OPP table which has dst_table as one of its required OPP table.
> > > + * @dst_table: Required OPP table of the src_table.
> > > + * @pstate: OPP of the src_table.
> > > + *
> > > + * This function returns the OPP (present in @dst_table) pointed out by the
> > > + * "required-opps" property of the OPP (present in @src_table).
> > > + *
> > > + * The callers are required to call dev_pm_opp_put() for the returned OPP after
> > > + * use.
> > > + *
> > > + * Return: destination table OPP on success, otherwise NULL on errors.
> > > + */
> > > +struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
> > > +                                       struct opp_table *dst_table,
> > > +                                       struct dev_pm_opp *src_opp)
> > > +{
> > > +       struct dev_pm_opp *opp, *dest_opp = NULL;
> > > +       int i;
> > > +
> > > +       if (!src_table || !dst_table || !src_opp)
> > > +               return NULL;
> > > +
> > > +       for (i = 0; i < src_table->required_opp_count; i++) {
> > > +               if (src_table->required_opp_tables[i]->np == dst_table->np)
> > > +                       break;
> > > +       }
> > > +
> > > +       if (unlikely(i == src_table->required_opp_count)) {
> > > +               pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
> > > +                      __func__, src_table, dst_table);
> > > +               return NULL;
> > > +       }
> > > +
> > > +       mutex_lock(&src_table->lock);
> > > +
> > > +       list_for_each_entry(opp, &src_table->opp_list, node) {
> > > +               if (opp == src_opp) {
> > > +                       dest_opp = opp->required_opps[i];
> >
> > Correct me if I am wrong. This patch assume that 'i' index is same on between
> > [1] and [2]. But in order to guarantee this assumption, all OPP entries
> > in the same opp_table have to have the same number of 'required-opps' properties
> > and keep the sequence among 'required-opps' entries.
> >
> > [1] src_table->required_opp_tables[i]->np
> > [2] opp->required_opps[I];
> >
> > For example, three OPP entries in the 'parent_bus_opp'
> > have the different sequence of 'required-opps' and the different
> > number of 'required-opps'. Is it no problem?
> >
> > parent_bus_opp: opp_table {
> >     compatible = "operating-points-v2";
> >
> >     opp2 {
> >         opp-hz = /bits/ 64 <200000>;
> >         required-opps = <&child_bus_a_opp2>, <&child_bus_b_opp2>,
> > <&child_bus_c_opp2>;
> >     };
> >
> >     opp1 {
> >         opp-hz = /bits/ 64 <200000>;
> >         // change the sequence between child_bus_b_opp2  and child_bus_c_opp2
> >         required-opps = <&child_bus_a_opp2>, <&child_bus_c_opp2>,
> > <&child_bus_b_opp2>
> >     };
> >
> >     opp0 {
> >         opp-hz = /bits/ 64 <200000>;
> >         // missing 'child_bus_a_opp2'
> >         required-opps = <&child_bus_c_opp2>, <&child_bus_b_opp2>
> >     };
> >
> > }
> >
>
> I get your question. If I'm not mistaken the OPP framework DT parsing
> code makes the assumption that the required-opps list has the phandles
> in the same order for each "row" in the OPP table. It actually only
> looks at the first OPP entry to figure out the list of required OPP
> tables.

Thanks for description. It is the limitation of 'required-opps' until now.

>
> Technically one can write code to deal with random order of the
> required-opp list, but doesn't seem like that's worth it because
> there's no need to have that order all mixed up in DT. And even if
> someone wants to add support for that, I don't think improving the DT
> parsing to handle random order would be part of this patch series.

I understand the existing ' required-opps' only consider the same sequence
of entries which are included in the same OPP table.

One more thing, 'required-opps' properties doesn't support
the other OPP enters of the different OPP table. Is it right of 'required-opps'?

Except for the random order, just each OPP might will requires
the different 'required-opps' of different OPP table. Even if it is
not related to random order, I think that this approach cannot
support them.

For example as following:
- opp2 used the OPP entries of 'child_bus_A' and 'child_bus_B' opp-table.
- opp1 used the OPP entries of 'child_bus_C' and 'child_bus_D' opp-table.

parent_bus_opp: opp_table {
    compatible = "operating-points-v2";

     opp2 {
         opp-hz = /bits/ 64 <200000>;
         required-opps = <&child_bus_A_opp2>, <&child_bus_B_opp2>;
    };

   opp1 {
         opp-hz = /bits/ 64 <200000>;
         required-opps = <&child_bus_C_opp0>, <&child_bus_D_opp0>;
    };
};
Saravana Kannan June 23, 2019, 6:07 a.m. UTC | #4
On Sat, Jun 22, 2019 at 9:28 PM Chanwoo Choi <cwchoi00@gmail.com> wrote:
>
> Hi,
>
> 2019년 6월 23일 (일) 오전 6:42, Saravana Kannan <saravanak@google.com>님이 작성:
> >
> > On Sat, Jun 22, 2019 at 4:50 AM Chanwoo Choi <cwchoi00@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > Absolutely, I like this approach. I think that it is necessary to make
> > > the connection
> > > between frequencies of devices.
> >
> > Happy to hear that.
> >
> > > But, I have a question on below.
> > >
> > > 2019년 6월 22일 (토) 오전 9:35, Saravana Kannan <saravanak@google.com>님이 작성:
> > > >
> > > > Add a function that allows looking up required OPPs given a source OPP
> > > > table, destination OPP table and the source OPP.
> > > >
> > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > > ---
> > > >  drivers/opp/core.c     | 54 ++++++++++++++++++++++++++++++++++++++++++
> > > >  include/linux/pm_opp.h | 11 +++++++++
> > > >  2 files changed, 65 insertions(+)
> > > >
> > > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> > > > index 74c7bdc6f463..4f7870bffbf8 100644
> > > > --- a/drivers/opp/core.c
> > > > +++ b/drivers/opp/core.c
> > > > @@ -1830,6 +1830,60 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table,
> > > >                 dev_err(virt_dev, "Failed to find required device entry\n");
> > > >  }
> > > >
> > > > +/**
> > > > + * dev_pm_opp_xlate_opp() - Find required OPP for src_table OPP.
> > > > + * @src_table: OPP table which has dst_table as one of its required OPP table.
> > > > + * @dst_table: Required OPP table of the src_table.
> > > > + * @pstate: OPP of the src_table.
> > > > + *
> > > > + * This function returns the OPP (present in @dst_table) pointed out by the
> > > > + * "required-opps" property of the OPP (present in @src_table).
> > > > + *
> > > > + * The callers are required to call dev_pm_opp_put() for the returned OPP after
> > > > + * use.
> > > > + *
> > > > + * Return: destination table OPP on success, otherwise NULL on errors.
> > > > + */
> > > > +struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
> > > > +                                       struct opp_table *dst_table,
> > > > +                                       struct dev_pm_opp *src_opp)
> > > > +{
> > > > +       struct dev_pm_opp *opp, *dest_opp = NULL;
> > > > +       int i;
> > > > +
> > > > +       if (!src_table || !dst_table || !src_opp)
> > > > +               return NULL;
> > > > +
> > > > +       for (i = 0; i < src_table->required_opp_count; i++) {
> > > > +               if (src_table->required_opp_tables[i]->np == dst_table->np)
> > > > +                       break;
> > > > +       }
> > > > +
> > > > +       if (unlikely(i == src_table->required_opp_count)) {
> > > > +               pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
> > > > +                      __func__, src_table, dst_table);
> > > > +               return NULL;
> > > > +       }
> > > > +
> > > > +       mutex_lock(&src_table->lock);
> > > > +
> > > > +       list_for_each_entry(opp, &src_table->opp_list, node) {
> > > > +               if (opp == src_opp) {
> > > > +                       dest_opp = opp->required_opps[i];
> > >
> > > Correct me if I am wrong. This patch assume that 'i' index is same on between
> > > [1] and [2]. But in order to guarantee this assumption, all OPP entries
> > > in the same opp_table have to have the same number of 'required-opps' properties
> > > and keep the sequence among 'required-opps' entries.
> > >
> > > [1] src_table->required_opp_tables[i]->np
> > > [2] opp->required_opps[I];
> > >
> > > For example, three OPP entries in the 'parent_bus_opp'
> > > have the different sequence of 'required-opps' and the different
> > > number of 'required-opps'. Is it no problem?
> > >
> > > parent_bus_opp: opp_table {
> > >     compatible = "operating-points-v2";
> > >
> > >     opp2 {
> > >         opp-hz = /bits/ 64 <200000>;
> > >         required-opps = <&child_bus_a_opp2>, <&child_bus_b_opp2>,
> > > <&child_bus_c_opp2>;
> > >     };
> > >
> > >     opp1 {
> > >         opp-hz = /bits/ 64 <200000>;
> > >         // change the sequence between child_bus_b_opp2  and child_bus_c_opp2
> > >         required-opps = <&child_bus_a_opp2>, <&child_bus_c_opp2>,
> > > <&child_bus_b_opp2>
> > >     };
> > >
> > >     opp0 {
> > >         opp-hz = /bits/ 64 <200000>;
> > >         // missing 'child_bus_a_opp2'
> > >         required-opps = <&child_bus_c_opp2>, <&child_bus_b_opp2>
> > >     };
> > >
> > > }
> > >
> >
> > I get your question. If I'm not mistaken the OPP framework DT parsing
> > code makes the assumption that the required-opps list has the phandles
> > in the same order for each "row" in the OPP table. It actually only
> > looks at the first OPP entry to figure out the list of required OPP
> > tables.
>
> Thanks for description. It is the limitation of 'required-opps' until now.
>
> >
> > Technically one can write code to deal with random order of the
> > required-opp list, but doesn't seem like that's worth it because
> > there's no need to have that order all mixed up in DT. And even if
> > someone wants to add support for that, I don't think improving the DT
> > parsing to handle random order would be part of this patch series.
>
> I understand the existing ' required-opps' only consider the same sequence
> of entries which are included in the same OPP table.
>
> One more thing, 'required-opps' properties doesn't support
> the other OPP enters of the different OPP table. Is it right of 'required-opps'?

Not sure I fully understand the question.

> Except for the random order, just each OPP might will requires
> the different 'required-opps' of different OPP table. Even if it is
> not related to random order, I think that this approach cannot
> support them.
>
> For example as following:
> - opp2 used the OPP entries of 'child_bus_A' and 'child_bus_B' opp-table.
> - opp1 used the OPP entries of 'child_bus_C' and 'child_bus_D' opp-table.
>
> parent_bus_opp: opp_table {
>     compatible = "operating-points-v2";
>
>      opp2 {
>          opp-hz = /bits/ 64 <200000>;

I'm guessing this is a typo and let's assume you meant to sat 400000

>          required-opps = <&child_bus_A_opp2>, <&child_bus_B_opp2>;
>     };
>
>    opp1 {
>          opp-hz = /bits/ 64 <200000>;
>          required-opps = <&child_bus_C_opp0>, <&child_bus_D_opp0>;
>     };
> };

Is this a real use case? If it is, in reality parent_bus_opp_table
always has requirements on all 4 children bus, just that opp1 is okay
with the lowest frequency for some of the children?

So, in this example, you just need to always list all 4 child OPPs for
each parent OPP. And some of the children OPP values might not change
when going from one parent OPP to another.

-Saravana
diff mbox series

Patch

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 74c7bdc6f463..4f7870bffbf8 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1830,6 +1830,60 @@  void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table,
 		dev_err(virt_dev, "Failed to find required device entry\n");
 }
 
+/**
+ * dev_pm_opp_xlate_opp() - Find required OPP for src_table OPP.
+ * @src_table: OPP table which has dst_table as one of its required OPP table.
+ * @dst_table: Required OPP table of the src_table.
+ * @pstate: OPP of the src_table.
+ *
+ * This function returns the OPP (present in @dst_table) pointed out by the
+ * "required-opps" property of the OPP (present in @src_table).
+ *
+ * The callers are required to call dev_pm_opp_put() for the returned OPP after
+ * use.
+ *
+ * Return: destination table OPP on success, otherwise NULL on errors.
+ */
+struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
+					struct opp_table *dst_table,
+					struct dev_pm_opp *src_opp)
+{
+	struct dev_pm_opp *opp, *dest_opp = NULL;
+	int i;
+
+	if (!src_table || !dst_table || !src_opp)
+		return NULL;
+
+	for (i = 0; i < src_table->required_opp_count; i++) {
+		if (src_table->required_opp_tables[i]->np == dst_table->np)
+			break;
+	}
+
+	if (unlikely(i == src_table->required_opp_count)) {
+		pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
+		       __func__, src_table, dst_table);
+		return NULL;
+	}
+
+	mutex_lock(&src_table->lock);
+
+	list_for_each_entry(opp, &src_table->opp_list, node) {
+		if (opp == src_opp) {
+			dest_opp = opp->required_opps[i];
+			dev_pm_opp_get(dest_opp);
+			goto unlock;
+		}
+	}
+
+	pr_err("%s: Couldn't find matching OPP (%p: %p)\n", __func__, src_table,
+	       dst_table);
+
+unlock:
+	mutex_unlock(&src_table->lock);
+
+	return dest_opp;
+}
+
 /**
  * dev_pm_opp_xlate_performance_state() - Find required OPP's pstate for src_table.
  * @src_table: OPP table which has dst_table as one of its required OPP table.
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index b150fe97ce5a..bc5c68bdfc8d 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -134,6 +134,9 @@  void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table);
 struct opp_table *dev_pm_opp_set_genpd_virt_dev(struct device *dev, struct device *virt_dev, int index);
 void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table, struct device *virt_dev);
 int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
+struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
+					struct opp_table *dst_table,
+					struct dev_pm_opp *src_opp);
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
 int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
 int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
@@ -307,6 +310,14 @@  static inline int dev_pm_opp_xlate_performance_state(struct opp_table *src_table
 	return -ENOTSUPP;
 }
 
+static inline struct dev_pm_opp *dev_pm_opp_xlate_opp(
+						struct opp_table *src_table,
+						struct opp_table *dst_table,
+						struct dev_pm_opp *src_opp)
+{
+	return NULL;
+}
+
 static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 {
 	return -ENOTSUPP;