On Fri, 29 Oct 2021 at 17:20, Dmitry Osipenko <digetx@xxxxxxxxx> wrote: > > 26.10.2021 01:40, Dmitry Osipenko пишет: > > + ret = devm_pm_runtime_enable(&pdev->dev); > > + if (ret) > > + return ret; > > + > > + ret = devm_tegra_core_dev_init_opp_table_common(&pdev->dev); > > + if (ret) > > + return ret; > > + > > + ret = pm_runtime_resume_and_get(&pdev->dev); > > + if (ret) > > + return ret; > > + > > /* Set maximum frequency of the IP */ > > - ret = clk_set_rate(pwm->clk, pwm->soc->max_frequency); > > + ret = dev_pm_opp_set_rate(pwm->dev, pwm->soc->max_frequency); > > if (ret < 0) { > > dev_err(&pdev->dev, "Failed to set max frequency: %d\n", ret); > > - return ret; > > + goto put_pm; > > } > > > > /* > > @@ -278,7 +294,7 @@ static int tegra_pwm_probe(struct platform_device *pdev) > > if (IS_ERR(pwm->rst)) { > > ret = PTR_ERR(pwm->rst); > > dev_err(&pdev->dev, "Reset control is not found: %d\n", ret); > > - return ret; > > + goto put_pm; > > } > > > > reset_control_deassert(pwm->rst); > > @@ -291,10 +307,15 @@ static int tegra_pwm_probe(struct platform_device *pdev) > > if (ret < 0) { > > dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret); > > reset_control_assert(pwm->rst); > > - return ret; > > + goto put_pm; > > } > > > > + pm_runtime_put(&pdev->dev); > > + > > return 0; > > +put_pm: > > + pm_runtime_put_sync_suspend(&pdev->dev); > > + return ret; > > } > > > > static int tegra_pwm_remove(struct platform_device *pdev) > > @@ -305,20 +326,44 @@ static int tegra_pwm_remove(struct platform_device *pdev) > > > > reset_control_assert(pc->rst); > > > > + pm_runtime_force_suspend(&pdev->dev); > > I just noticed that RPM core doesn't reset RPM-enable count of a device > on driver's unbind (pm_runtime_reinit). It was a bad idea to use > devm_pm_runtime_enable() + pm_runtime_force_suspend() here, since RPM is > disabled twice on driver's removal, and thus, RPM will never be enabled > again. > > I'll fix it for PWM and other drivers in this series, in v15. Good catch - and sorry for not spotting it while reviewing! Maybe devm_pm_runtime_enable() isn't that useful after all? Should we suggest to remove it so others don't fall into the same trap? Kind regards Uffe