10.08.2021 01:44, Dmitry Osipenko пишет: > 10.08.2021 01:26, Dmitry Osipenko пишет: >> 04.08.2021 13:58, Rajendra Nayak пишет: >>> @@ -2637,6 +2643,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >>> { >>> struct of_phandle_args pd_args; >>> struct generic_pm_domain *pd; >>> + struct device_node *np; >>> + int pstate; >>> int ret; >>> >>> ret = of_parse_phandle_with_args(dev->of_node, "power-domains", >>> @@ -2675,10 +2683,26 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, >>> genpd_unlock(pd); >>> } >>> >>> - if (ret) >>> + if (ret) { >>> genpd_remove_device(pd, dev); >>> + return -EPROBE_DEFER; >>> + } >>> + >>> + /* Set the default performance state */ >>> + np = dev->of_node; >>> + if (of_parse_phandle(np, "required-opps", index)) { The OF node returned by of_parse_phandle() must be put. >>> + pstate = of_get_required_opp_performance_state(np, index); If you have more than one power domain, then this will override the pstate which was set for a previous domain. This code doesn't feel solid to me, at least a clarifying comment is needed about how it's supposed to work. >>> + if (pstate < 0) { >>> + ret = pstate; >>> + dev_err(dev, "failed to set required performance state for power-domain %s: %d\n", >>> + pd->name, ret); >>> + } else { >>> + dev_pm_genpd_set_performance_state(dev, pstate); > > Where is error handling? > >>> + dev_gpd_data(dev)->default_pstate = pstate; >>> + } >>> + } >> >> Why performance state is set after genpd_power_on()? Should set_performance_state() be invoked when power_on=false? I assume it should be: if (power_on) { dev_pm_genpd_set_performance_state(dev, pstate); dev_gpd_data(dev)->default_pstate = pstate; } else { dev_gpd_data(dev)->rpm_pstate = pstate; }