Re: [PATCH v7 02/37] soc/tegra: pmc: Implement attach_dev() of power domain drivers

[Dmitry Osipenko <digetx@gmail.com>]

02.08.2021 17:48, Ulf Hansson пишет:
...
>> +       if (!list_empty(&genpd->child_links)) {
>> +               link = list_first_entry(&genpd->child_links, struct gpd_link,
>> +                                       child_node);
>> +               core_genpd = link->parent;
>> +       } else {
>> +               core_genpd = genpd;
>> +       }
> 
> This looks a bit odd to me. A genpd provider shouldn't need to walk
> these links as these are considered internals to genpd. Normally this
> needs lockings, etc.
> 
> Why exactly do you need this?

We have a chain of PMC domain -> core domain, both domains are created
and liked together by this PMC driver. Devices are attached to either
PMC domain or to core domain. PMC domain doesn't handle the performance
changes, performance requests go down to the core domain.

This is needed in order to translate the device's OPP into performance
state of the core domain, based on the domain to which device is attached.

>> +
>> +       pd_opp_table = dev_pm_opp_get_opp_table(&core_genpd->dev);
>> +       if (IS_ERR(pd_opp_table)) {
>> +               dev_err(&genpd->dev, "failed to get OPP table of %s: %pe\n",
>> +                       dev_name(&core_genpd->dev), pd_opp_table);
>> +               ret = PTR_ERR(pd_opp_table);
>> +               goto put_dev_opp;
>> +       }
>> +
>> +       pd_opp = dev_pm_opp_xlate_required_opp(opp_table, pd_opp_table, opp);
>> +       if (IS_ERR(pd_opp)) {
>> +               dev_err(&genpd->dev,
>> +                       "failed to xlate required OPP for %luHz of %s: %pe\n",
>> +                       rate, dev_name(dev), pd_opp);
>> +               ret = PTR_ERR(pd_opp);
>> +               goto put_pd_opp_table;
>> +       }
>> +
>> +       /*
>> +        * The initialized state will be applied by GENPD core on the first
>> +        * RPM-resume of the device.  This means that drivers don't need to
>> +        * explicitly initialize performance state.
>> +        */
>> +       state = pm_genpd_opp_to_performance_state(&core_genpd->dev, pd_opp);
>> +       gpd_data->rpm_pstate = state;
> 
> Could the above be replaced with Rajendra's suggestion [1], which
> changes genpd to internally during attach, to set a default
> performance state when there is a "required-opp" specified in the
> device  node?

It's not a "static" performance level here, but any level depending on
h/w state left from bootloader and etc. The performance level
corresponds to the voltage of the core domain, hence we need to
initialize the voltage vote before device is resumed.