On Mon, May 13, 2019 at 03:54:11PM +0530, Viresh Kumar wrote:
> Currently the space for the array of virtual devices is allocated along
> with the OPP table, but that isn't going to work well from now onwards.
> For single power domain case, a driver can either use the original
> device structure for setting the performance state (if genpd attached
> with dev_pm_domain_attach()) or use the virtual device structure (if
> genpd attached with dev_pm_domain_attach_by_name(), which returns the
> virtual device) and so we can't know in advance if we are going to need
> genpd_virt_devs array or not.
> 
> Lets delay the allocation a bit and do it along with
> dev_pm_opp_attach_genpd() rather. The deallocation is done from
> dev_pm_opp_detach_genpd().
> 
> Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>
> ---
>  drivers/opp/core.c | 10 ++++++++++
>  drivers/opp/of.c   | 30 ++----------------------------
>  2 files changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 67d6b0caeab1..764e05a2fa66 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1755,6 +1755,9 @@ static void _opp_detach_genpd(struct opp_table 
> *opp_table)
>               dev_pm_domain_detach(opp_table->genpd_virt_devs[index], false);
>               opp_table->genpd_virt_devs[index] = NULL;
>       }
> +
> +     kfree(opp_table->genpd_virt_devs);
> +     opp_table->genpd_virt_devs = NULL;
>  }
>  
>  /**
> @@ -1798,6 +1801,12 @@ struct opp_table *dev_pm_opp_attach_genpd(struct 
> device *dev, const char **names
>  
>       mutex_lock(&opp_table->genpd_virt_dev_lock);
>  
> +     opp_table->genpd_virt_devs = kcalloc(opp_table->required_opp_count,
> +                                          
> sizeof(*opp_table->genpd_virt_devs),
> +                                          GFP_KERNEL);
> +     if (!opp_table->genpd_virt_devs)
> +             goto unlock;
> +
>       while (*name) {
>               index = of_property_match_string(dev->of_node,
>                                                "power-domain-names", *name);
> @@ -1836,6 +1845,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device 
> *dev, const char **names
>  
>  err:
>       _opp_detach_genpd(opp_table);
> +unlock:
>       mutex_unlock(&opp_table->genpd_virt_dev_lock);
>  
>  put_table:
> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> index c10c782d15aa..a637f30552a3 100644
> --- a/drivers/opp/of.c
> +++ b/drivers/opp/of.c
> @@ -141,7 +141,6 @@ static struct opp_table *_find_table_of_opp_np(struct 
> device_node *opp_np)
>  static void _opp_table_free_required_tables(struct opp_table *opp_table)
>  {
>       struct opp_table **required_opp_tables = opp_table->required_opp_tables;
> -     struct device **genpd_virt_devs = opp_table->genpd_virt_devs;
>       int i;
>  
>       if (!required_opp_tables)
> @@ -155,10 +154,8 @@ static void _opp_table_free_required_tables(struct 
> opp_table *opp_table)
>       }
>  
>       kfree(required_opp_tables);
> -     kfree(genpd_virt_devs);
>  
>       opp_table->required_opp_count = 0;
> -     opp_table->genpd_virt_devs = NULL;
>       opp_table->required_opp_tables = NULL;
>  }
>  
> @@ -171,9 +168,8 @@ static void _opp_table_alloc_required_tables(struct 
> opp_table *opp_table,
>                                            struct device_node *opp_np)
>  {
>       struct opp_table **required_opp_tables;
> -     struct device **genpd_virt_devs = NULL;
>       struct device_node *required_np, *np;
> -     int count, count_pd, i;
> +     int count, i;
>  
>       /* Traversing the first OPP node is all we need */
>       np = of_get_next_available_child(opp_np, NULL);
> @@ -186,33 +182,11 @@ static void _opp_table_alloc_required_tables(struct 
> opp_table *opp_table,
>       if (!count)
>               goto put_np;
>  
> -     /*
> -      * Check the number of power-domains to know if we need to deal
> -      * with virtual devices. In some cases we have devices with multiple
> -      * power domains but with only one of them being scalable, hence
> -      * 'count' could be 1, but we still have to deal with multiple genpds
> -      * and virtual devices.
> -      */
> -     count_pd = of_count_phandle_with_args(dev->of_node, "power-domains",
> -                                           "#power-domain-cells");
> -     if (!count_pd)
> -             goto put_np;
> -
> -     if (count_pd > 1) {
> -             genpd_virt_devs = kcalloc(count, sizeof(*genpd_virt_devs),
> -                                     GFP_KERNEL);
> -             if (!genpd_virt_devs)
> -                     goto put_np;
> -     }
> -
>       required_opp_tables = kcalloc(count, sizeof(*required_opp_tables),
>                                     GFP_KERNEL);
> -     if (!required_opp_tables) {
> -             kfree(genpd_virt_devs);
> +     if (!required_opp_tables)
>               goto put_np;
> -     }
>  
> -     opp_table->genpd_virt_devs = genpd_virt_devs;
>       opp_table->required_opp_tables = required_opp_tables;
>       opp_table->required_opp_count = count;
>  
> -- 
> 2.21.0.rc0.269.g1a574e7a288b
> 

Tested-by: Niklas Cassel <niklas.cas...@linaro.org>

Reply via email to