16.04.2019 4:52, Chanwoo Choi пишет: > Hi, > > On 19. 4. 15. 오후 11:54, Dmitry Osipenko wrote: >> The clk_set_min_rate() could fail and in this case clk_set_rate() sets >> rate to 0, which may drop EMC rate to minimum and make machine very >> difficult to use. >> >> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx> >> --- >> drivers/devfreq/tegra-devfreq.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/devfreq/tegra-devfreq.c b/drivers/devfreq/tegra-devfreq.c >> index 0c0909fba545..69581c9082d4 100644 >> --- a/drivers/devfreq/tegra-devfreq.c >> +++ b/drivers/devfreq/tegra-devfreq.c >> @@ -487,6 +487,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq, >> struct tegra_devfreq *tegra = dev_get_drvdata(dev); >> struct dev_pm_opp *opp; >> unsigned long rate; >> + int err; >> >> opp = devfreq_recommended_opp(dev, freq, flags); >> if (IS_ERR(opp)) { >> @@ -496,8 +497,13 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq, >> rate = dev_pm_opp_get_freq(opp); >> dev_pm_opp_put(opp); >> >> - clk_set_min_rate(tegra->emc_clock, rate); >> - clk_set_rate(tegra->emc_clock, 0); >> + err = clk_set_min_rate(tegra->emc_clock, rate); >> + if (err) >> + return err; >> + >> + err = clk_set_rate(tegra->emc_clock, 0); >> + if (err) > > Why don't you restore the previous minimum clock for tegra->emc_clock > when failed to execute 'clk_set_rate(tegra->emc_clock, 0);'? > > I think that if you control the restoring the previous minimum clock, > it is more nice to handle the error handling. I now see that devfreq has the previous_freq field and indeed it is possible to restore the min_rate value using it. Thanks for the suggestion.